| Message ID | 20240729110454.346918-3-thorsten.blum@toblux.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: Annotate struct ext4_xattr_inode_array with __counted_by() | expand |
On 29/07/24 05:04, Thorsten Blum wrote: > Add the __counted_by compiler attribute to the flexible array member > inodes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and > CONFIG_FORTIFY_SOURCE. This change seems to be incomplete. The relationship between `count` and accesses to `inodes` should be adjusted at least in `ext4_expand_inode_array()` See this for more details: https://embeddedor.com/blog/2024/06/18/how-to-use-the-new-counted_by-attribute-in-c-and-linux/ Thanks -- Gustavo > > Remove the now obsolete comment on the count field. > > Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> > --- > fs/ext4/xattr.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h > index bd97c4aa8177..e14fb19dc912 100644 > --- a/fs/ext4/xattr.h > +++ b/fs/ext4/xattr.h > @@ -130,8 +130,8 @@ struct ext4_xattr_ibody_find { > }; > > struct ext4_xattr_inode_array { > - unsigned int count; /* # of used items in the array */ > - struct inode *inodes[]; > + unsigned int count; > + struct inode *inodes[] __counted_by(count); > }; > > extern const struct xattr_handler ext4_xattr_user_handler;
On 2024/7/29 19:04, Thorsten Blum wrote: > Add the __counted_by compiler attribute to the flexible array member > inodes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and > CONFIG_FORTIFY_SOURCE. > > Remove the now obsolete comment on the count field. > > Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> > --- > fs/ext4/xattr.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h > index bd97c4aa8177..e14fb19dc912 100644 > --- a/fs/ext4/xattr.h > +++ b/fs/ext4/xattr.h > @@ -130,8 +130,8 @@ struct ext4_xattr_ibody_find { > }; > > struct ext4_xattr_inode_array { > - unsigned int count; /* # of used items in the array */ > - struct inode *inodes[]; > + unsigned int count; As the comment says, 'count' is the number of items in the array that have been used, not the total number of items in the array. So I think this check was added incorrectly. > + struct inode *inodes[] __counted_by(count); > }; > > extern const struct xattr_handler ext4_xattr_user_handler;
On 29. Jul 2024, at 16:09, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > On 29/07/24 05:04, Thorsten Blum wrote: >> Add the __counted_by compiler attribute to the flexible array member >> inodes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and >> CONFIG_FORTIFY_SOURCE. > > This change seems to be incomplete. The relationship between `count` and > accesses to `inodes` should be adjusted at least in `ext4_expand_inode_array()` > > See this for more details: > > https://embeddedor.com/blog/2024/06/18/how-to-use-the-new-counted_by-attribute-in-c-and-linux/ Thanks. I'll submit a v2 shortly. Thorsten
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h index bd97c4aa8177..e14fb19dc912 100644 --- a/fs/ext4/xattr.h +++ b/fs/ext4/xattr.h @@ -130,8 +130,8 @@ struct ext4_xattr_ibody_find { }; struct ext4_xattr_inode_array { - unsigned int count; /* # of used items in the array */ - struct inode *inodes[]; + unsigned int count; + struct inode *inodes[] __counted_by(count); }; extern const struct xattr_handler ext4_xattr_user_handler;
Add the __counted_by compiler attribute to the flexible array member inodes to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE. Remove the now obsolete comment on the count field. Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com> --- fs/ext4/xattr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)