| Message ID | 1390829312-814-3-git-send-email-Gerhard@Heift.Name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 27, 2014 at 02:28:28PM +0100, Gerhard Heift wrote: > To prevent unexpectet values in the unused fields of the search key fail early. > Otherwise future extensions would break the behavior of the search if current > implementations in userspace set them to values other than zero. > > Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1947,6 +1947,9 @@ static noinline int search_ioctl(struct inode *inode, > if (buf_size < sizeof(struct btrfs_ioctl_search_header)) > return -EOVERFLOW; > > + if (sk->unused || sk->unused1 | sk->unused2 | sk->unused3 | sk->unused4) > + return -EINVAL; The pattern that's been used for forward/backward compatibility is to zero the unused or reserved fields on the userspace side and ignore them completely in kernel. If any future version of the ioctl uses the now unused fields, it also has to increase the version. > + > path = btrfs_alloc_path(); > if (!path) > return -ENOMEM; -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Montag, 27. Januar 2014, 14:28:28 schrieb Gerhard Heift: > To prevent unexpectet values in the unused fields of the search key fail unexpected > early. Otherwise future extensions would break the behavior of the search > if current implementations in userspace set them to values other than zero. > > Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> > --- > fs/btrfs/ioctl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index be4c780..919d928 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1947,6 +1947,9 @@ static noinline int search_ioctl(struct inode *inode, > if (buf_size < sizeof(struct btrfs_ioctl_search_header)) > return -EOVERFLOW; > > + if (sk->unused || sk->unused1 | sk->unused2 | sk->unused3 | sk->unused4) > + return -EINVAL; > + > path = btrfs_alloc_path(); > if (!path) > return -ENOMEM;
2014-01-27 David Sterba <dsterba@suse.cz>: > On Mon, Jan 27, 2014 at 02:28:28PM +0100, Gerhard Heift wrote: >> To prevent unexpectet values in the unused fields of the search key fail early. >> Otherwise future extensions would break the behavior of the search if current >> implementations in userspace set them to values other than zero. >> >> Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -1947,6 +1947,9 @@ static noinline int search_ioctl(struct inode *inode, >> if (buf_size < sizeof(struct btrfs_ioctl_search_header)) >> return -EOVERFLOW; >> >> + if (sk->unused || sk->unused1 | sk->unused2 | sk->unused3 | sk->unused4) >> + return -EINVAL; > > The pattern that's been used for forward/backward compatibility is to > zero the unused or reserved fields on the userspace side and ignore them > completely in kernel. OK, I will dismiss this patch. > If any future version of the ioctl uses the now unused fields, it also > has to increase the version. Just for me to learn: If we have to increase the version, why do we have the unused fields anyway? We could expand the struct on demand if we need new fields. Do I miss something? -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 28, 2014 at 01:32:36AM +0100, Gerhard Heift wrote: > > If any future version of the ioctl uses the now unused fields, it also > > has to increase the version. > > Just for me to learn: If we have to increase the version, why do we > have the unused fields anyway? We could expand the struct on demand if > we need new fields. Do I miss something? The spare fields can be used for minor updates, eg. returning a simple number or status information, or passing some flags down to the ioctl. Your patch does a major change to the crucial member of the ioctl structure, the existing unused fileds don't help here and will stay unused. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index be4c780..919d928 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1947,6 +1947,9 @@ static noinline int search_ioctl(struct inode *inode, if (buf_size < sizeof(struct btrfs_ioctl_search_header)) return -EOVERFLOW; + if (sk->unused || sk->unused1 | sk->unused2 | sk->unused3 | sk->unused4) + return -EINVAL; + path = btrfs_alloc_path(); if (!path) return -ENOMEM;
To prevent unexpectet values in the unused fields of the search key fail early. Otherwise future extensions would break the behavior of the search if current implementations in userspace set them to values other than zero. Signed-off-by: Gerhard Heift <Gerhard@Heift.Name> --- fs/btrfs/ioctl.c | 3 +++ 1 file changed, 3 insertions(+)