* [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL @ 2019-08-16 5:55 Eric Biggers 2019-08-16 5:55 ` [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL Eric Biggers ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Eric Biggers @ 2019-08-16 5:55 UTC (permalink / raw) To: linux-f2fs-devel Fix some bugs in the f2fs implementation of the FS_IOC_GETFSLABEL and FS_IOC_SETFSLABEL ioctls. Eric Biggers (3): f2fs: fix buffer overruns in FS_IOC_{GET,SET}FSLABEL f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL f2fs: add missing authorization check in FS_IOC_SETFSLABEL fs/f2fs/file.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) -- 2.22.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL 2019-08-16 5:55 [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Eric Biggers @ 2019-08-16 5:55 ` Eric Biggers 2019-08-16 6:50 ` Chao Yu 2019-08-16 5:55 ` [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL Eric Biggers ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-16 5:55 UTC (permalink / raw) To: linux-f2fs-devel From: Eric Biggers <ebiggers@google.com> utf16s_to_utf8s() and utf8s_to_utf16s() take the number of characters, not the number of bytes. Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/f2fs/file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index eb1aa9b75eda..d521a582d94d 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3094,7 +3094,7 @@ static int f2fs_get_volume_name(struct file *filp, unsigned long arg) down_read(&sbi->sb_lock); count = utf16s_to_utf8s(sbi->raw_super->volume_name, - sizeof(sbi->raw_super->volume_name), + ARRAY_SIZE(sbi->raw_super->volume_name), UTF16_LITTLE_ENDIAN, vbuf, MAX_VOLUME_NAME); up_read(&sbi->sb_lock); @@ -3139,7 +3139,7 @@ static int f2fs_set_volume_name(struct file *filp, unsigned long arg) sizeof(sbi->raw_super->volume_name)); utf8s_to_utf16s(vbuf, MAX_VOLUME_NAME, UTF16_LITTLE_ENDIAN, sbi->raw_super->volume_name, - sizeof(sbi->raw_super->volume_name)); + ARRAY_SIZE(sbi->raw_super->volume_name)); err = f2fs_commit_super(sbi, false); -- 2.22.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL 2019-08-16 5:55 ` [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL Eric Biggers @ 2019-08-16 6:50 ` Chao Yu 0 siblings, 0 replies; 16+ messages in thread From: Chao Yu @ 2019-08-16 6:50 UTC (permalink / raw) To: Eric Biggers, linux-f2fs-devel On 2019/8/16 13:55, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > utf16s_to_utf8s() and utf8s_to_utf16s() take the number of characters, > not the number of bytes. > > Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") > Signed-off-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-16 5:55 [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Eric Biggers 2019-08-16 5:55 ` [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL Eric Biggers @ 2019-08-16 5:55 ` Eric Biggers 2019-08-16 6:59 ` Chao Yu 2019-08-16 5:55 ` [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check " Eric Biggers 2019-08-16 6:49 ` [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Chao Yu 3 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-16 5:55 UTC (permalink / raw) To: linux-f2fs-devel From: Eric Biggers <ebiggers@google.com> Userspace provides a null-terminated string, so don't assume that the full FSLABEL_MAX bytes can always be copied. Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/f2fs/file.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index d521a582d94d..315127251bc1 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3111,23 +3111,11 @@ static int f2fs_set_volume_name(struct file *filp, unsigned long arg) struct inode *inode = file_inode(filp); struct f2fs_sb_info *sbi = F2FS_I_SB(inode); char *vbuf; - int len; int err = 0; - vbuf = f2fs_kzalloc(sbi, MAX_VOLUME_NAME, GFP_KERNEL); - if (!vbuf) - return -ENOMEM; - - if (copy_from_user(vbuf, (char __user *)arg, FSLABEL_MAX)) { - err = -EFAULT; - goto out; - } - - len = strnlen(vbuf, FSLABEL_MAX); - if (len > FSLABEL_MAX - 1) { - err = -EINVAL; - goto out; - } + vbuf = strndup_user((const char __user *)arg, FSLABEL_MAX); + if (IS_ERR(vbuf)) + return PTR_ERR(vbuf); err = mnt_want_write_file(filp); if (err) @@ -3137,7 +3125,7 @@ static int f2fs_set_volume_name(struct file *filp, unsigned long arg) memset(sbi->raw_super->volume_name, 0, sizeof(sbi->raw_super->volume_name)); - utf8s_to_utf16s(vbuf, MAX_VOLUME_NAME, UTF16_LITTLE_ENDIAN, + utf8s_to_utf16s(vbuf, strlen(vbuf), UTF16_LITTLE_ENDIAN, sbi->raw_super->volume_name, ARRAY_SIZE(sbi->raw_super->volume_name)); @@ -3147,7 +3135,7 @@ static int f2fs_set_volume_name(struct file *filp, unsigned long arg) mnt_drop_write_file(filp); out: - kvfree(vbuf); + kfree(vbuf); return err; } -- 2.22.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-16 5:55 ` [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL Eric Biggers @ 2019-08-16 6:59 ` Chao Yu 2019-08-18 15:41 ` Eric Biggers 0 siblings, 1 reply; 16+ messages in thread From: Chao Yu @ 2019-08-16 6:59 UTC (permalink / raw) To: Eric Biggers, linux-f2fs-devel On 2019/8/16 13:55, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Userspace provides a null-terminated string, so don't assume that the > full FSLABEL_MAX bytes can always be copied.> > Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") It may only copy redundant zero bytes, and will not hit security issue, it doesn't look like a bug fix? > Signed-off-by: Eric Biggers <ebiggers@google.com> Anyway, it makes sense to me. Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-16 6:59 ` Chao Yu @ 2019-08-18 15:41 ` Eric Biggers 2019-08-19 1:33 ` Chao Yu 0 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-18 15:41 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote: > On 2019/8/16 13:55, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > Userspace provides a null-terminated string, so don't assume that the > > full FSLABEL_MAX bytes can always be copied.> > > Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") > > It may only copy redundant zero bytes, and will not hit security issue, it > doesn't look like a bug fix? > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > Anyway, it makes sense to me. > > Reviewed-by: Chao Yu <yuchao0@huawei.com> > It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX bytes in the buffer. E.g. it could provide "foo\0" followed by an unmapped page. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-18 15:41 ` Eric Biggers @ 2019-08-19 1:33 ` Chao Yu 2019-08-19 1:58 ` Chao Yu 0 siblings, 1 reply; 16+ messages in thread From: Chao Yu @ 2019-08-19 1:33 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-f2fs-devel On 2019/8/18 23:41, Eric Biggers wrote: > On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote: >> On 2019/8/16 13:55, Eric Biggers wrote: >>> From: Eric Biggers <ebiggers@google.com> >>> >>> Userspace provides a null-terminated string, so don't assume that the >>> full FSLABEL_MAX bytes can always be copied.> >>> Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") >> >> It may only copy redundant zero bytes, and will not hit security issue, it >> doesn't look like a bug fix? >> >>> Signed-off-by: Eric Biggers <ebiggers@google.com> >> >> Anyway, it makes sense to me. >> >> Reviewed-by: Chao Yu <yuchao0@huawei.com> >> > > It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX bytes > in the buffer. E.g. it could provide "foo\0" followed by an unmapped page. You're right, thanks for your explanation. Thanks, > > - Eric > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-19 1:33 ` Chao Yu @ 2019-08-19 1:58 ` Chao Yu 2019-08-19 2:55 ` Eric Biggers 0 siblings, 1 reply; 16+ messages in thread From: Chao Yu @ 2019-08-19 1:58 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-f2fs-devel On 2019/8/19 9:33, Chao Yu wrote: > On 2019/8/18 23:41, Eric Biggers wrote: >> On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote: >>> On 2019/8/16 13:55, Eric Biggers wrote: >>>> From: Eric Biggers <ebiggers@google.com> >>>> >>>> Userspace provides a null-terminated string, so don't assume that the >>>> full FSLABEL_MAX bytes can always be copied.> >>>> Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") >>> >>> It may only copy redundant zero bytes, and will not hit security issue, it >>> doesn't look like a bug fix? >>> >>>> Signed-off-by: Eric Biggers <ebiggers@google.com> >>> >>> Anyway, it makes sense to me. >>> >>> Reviewed-by: Chao Yu <yuchao0@huawei.com> >>> >> >> It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX bytes >> in the buffer. E.g. it could provide "foo\0" followed by an unmapped page. > > You're right, thanks for your explanation. One more question, there is no validation check on length of user passed buffer, So in most ioctl interfaces, user can pass a buffer which has less size than we defined intentionally/unintentionally. E.g. user space: struct f2fs_defragment_user { unsigned long long start; // unsigned long long len; }; main() { struct f2fs_defragment_user *df; df = malloc(); ioctl(fd, F2FS_IOC_DEFRAGMENT, df); } kernel: f2fs_ioc_defragment() { ... if (copy_from_user(&range, (struct f2fs_defragment __user *)arg, sizeof(range))) return -EFAULT; } Is that a common issue? Thanks, > > Thanks, > >> >> - Eric >> . >> > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-19 1:58 ` Chao Yu @ 2019-08-19 2:55 ` Eric Biggers 2019-08-19 3:24 ` Chao Yu 0 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-19 2:55 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel On Mon, Aug 19, 2019 at 09:58:30AM +0800, Chao Yu wrote: > On 2019/8/19 9:33, Chao Yu wrote: > > On 2019/8/18 23:41, Eric Biggers wrote: > >> On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote: > >>> On 2019/8/16 13:55, Eric Biggers wrote: > >>>> From: Eric Biggers <ebiggers@google.com> > >>>> > >>>> Userspace provides a null-terminated string, so don't assume that the > >>>> full FSLABEL_MAX bytes can always be copied.> > >>>> Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") > >>> > >>> It may only copy redundant zero bytes, and will not hit security issue, it > >>> doesn't look like a bug fix? > >>> > >>>> Signed-off-by: Eric Biggers <ebiggers@google.com> > >>> > >>> Anyway, it makes sense to me. > >>> > >>> Reviewed-by: Chao Yu <yuchao0@huawei.com> > >>> > >> > >> It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX bytes > >> in the buffer. E.g. it could provide "foo\0" followed by an unmapped page. > > > > You're right, thanks for your explanation. > > One more question, there is no validation check on length of user passed buffer, > > So in most ioctl interfaces, user can pass a buffer which has less size than we > defined intentionally/unintentionally. > > E.g. > > user space: > > struct f2fs_defragment_user { > unsigned long long start; > // unsigned long long len; > }; > > main() > { > struct f2fs_defragment_user *df; > > df = malloc(); > > ioctl(fd, F2FS_IOC_DEFRAGMENT, df); > } > > kernel: > > f2fs_ioc_defragment() > { > ... > if (copy_from_user(&range, (struct f2fs_defragment __user *)arg, > sizeof(range))) > return -EFAULT; > } > > Is that a common issue? > No, but that's different because that only involves a fixed-length struct. My concern was that since FS_IOC_SETFSLABEL takes in a string, users might do: ioctl(fd, FS_IOC_SETFSLABEL, "foo"); Rather than: char label[FSLABEL_MAX] = "foo"; ioctl(fd, FS_IOC_SETFSLABEL, label); At least that's how I understand the ioctl; AFAICS it does not have a man page, so I'm not sure what was intended. Assuming the buffer is always FSLABEL_MAX bytes seems like a really bad idea though, since if users pass a conventional string (as is the natural thing to do; open() doesn't require a buffer of length PATH_MAX, for example...) it will succeed/fail at random depending on whether the following page is mapped or not. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL 2019-08-19 2:55 ` Eric Biggers @ 2019-08-19 3:24 ` Chao Yu 0 siblings, 0 replies; 16+ messages in thread From: Chao Yu @ 2019-08-19 3:24 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-f2fs-devel On 2019/8/19 10:55, Eric Biggers wrote: > On Mon, Aug 19, 2019 at 09:58:30AM +0800, Chao Yu wrote: >> On 2019/8/19 9:33, Chao Yu wrote: >>> On 2019/8/18 23:41, Eric Biggers wrote: >>>> On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote: >>>>> On 2019/8/16 13:55, Eric Biggers wrote: >>>>>> From: Eric Biggers <ebiggers@google.com> >>>>>> >>>>>> Userspace provides a null-terminated string, so don't assume that the >>>>>> full FSLABEL_MAX bytes can always be copied.> >>>>>> Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") >>>>> >>>>> It may only copy redundant zero bytes, and will not hit security issue, it >>>>> doesn't look like a bug fix? >>>>> >>>>>> Signed-off-by: Eric Biggers <ebiggers@google.com> >>>>> >>>>> Anyway, it makes sense to me. >>>>> >>>>> Reviewed-by: Chao Yu <yuchao0@huawei.com> >>>>> >>>> >>>> It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX bytes >>>> in the buffer. E.g. it could provide "foo\0" followed by an unmapped page. >>> >>> You're right, thanks for your explanation. >> >> One more question, there is no validation check on length of user passed buffer, >> >> So in most ioctl interfaces, user can pass a buffer which has less size than we >> defined intentionally/unintentionally. >> >> E.g. >> >> user space: >> >> struct f2fs_defragment_user { >> unsigned long long start; >> // unsigned long long len; >> }; >> >> main() >> { >> struct f2fs_defragment_user *df; >> >> df = malloc(); >> >> ioctl(fd, F2FS_IOC_DEFRAGMENT, df); >> } >> >> kernel: >> >> f2fs_ioc_defragment() >> { >> ... >> if (copy_from_user(&range, (struct f2fs_defragment __user *)arg, >> sizeof(range))) >> return -EFAULT; >> } >> >> Is that a common issue? >> > > No, but that's different because that only involves a fixed-length struct. > > My concern was that since FS_IOC_SETFSLABEL takes in a string, users might do: > > ioctl(fd, FS_IOC_SETFSLABEL, "foo"); Yes, I can understand your concern, since in this case, user's behavior is normal. But what I'm trying to say is, from the result aspect, when user pass a buffer which has less size intentionally/unintentionally (even kernel defines a fix-sized struture, but there is no rules that users can not reconstruct it or change its size as their wish), kernel may access unmapped page follows to user's buffer potentially. It needs to be fixed if that's a real problem. Thanks, > > Rather than: > > char label[FSLABEL_MAX] = "foo"; > > ioctl(fd, FS_IOC_SETFSLABEL, label); > > At least that's how I understand the ioctl; AFAICS it does not have a man page, > so I'm not sure what was intended. Assuming the buffer is always FSLABEL_MAX > bytes seems like a really bad idea though, since if users pass a conventional > string (as is the natural thing to do; open() doesn't require a buffer of length > PATH_MAX, for example...) it will succeed/fail at random depending on whether > the following page is mapped or not. > > - Eric > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check in FS_IOC_SETFSLABEL 2019-08-16 5:55 [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Eric Biggers 2019-08-16 5:55 ` [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL Eric Biggers 2019-08-16 5:55 ` [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL Eric Biggers @ 2019-08-16 5:55 ` Eric Biggers 2019-08-16 7:00 ` Chao Yu 2019-08-16 6:49 ` [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Chao Yu 3 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-16 5:55 UTC (permalink / raw) To: linux-f2fs-devel From: Eric Biggers <ebiggers@google.com> FS_IOC_SETFSLABEL modifies the filesystem superblock, so it shouldn't be allowed to regular users. Require CAP_SYS_ADMIN, like xfs and btrfs do. Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/f2fs/file.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 315127251bc1..6939cddc542a 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3113,6 +3113,9 @@ static int f2fs_set_volume_name(struct file *filp, unsigned long arg) char *vbuf; int err = 0; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + vbuf = strndup_user((const char __user *)arg, FSLABEL_MAX); if (IS_ERR(vbuf)) return PTR_ERR(vbuf); -- 2.22.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check in FS_IOC_SETFSLABEL 2019-08-16 5:55 ` [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check " Eric Biggers @ 2019-08-16 7:00 ` Chao Yu 0 siblings, 0 replies; 16+ messages in thread From: Chao Yu @ 2019-08-16 7:00 UTC (permalink / raw) To: Eric Biggers, linux-f2fs-devel On 2019/8/16 13:55, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > FS_IOC_SETFSLABEL modifies the filesystem superblock, so it shouldn't be > allowed to regular users. Require CAP_SYS_ADMIN, like xfs and btrfs do. > > Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL") > Signed-off-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL 2019-08-16 5:55 [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Eric Biggers ` (2 preceding siblings ...) 2019-08-16 5:55 ` [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check " Eric Biggers @ 2019-08-16 6:49 ` Chao Yu 2019-08-18 13:03 ` Chao Yu 3 siblings, 1 reply; 16+ messages in thread From: Chao Yu @ 2019-08-16 6:49 UTC (permalink / raw) To: Eric Biggers, linux-f2fs-devel Eric, thanks for all the fixes. Thanks, On 2019/8/16 13:55, Eric Biggers wrote: > Fix some bugs in the f2fs implementation of the FS_IOC_GETFSLABEL and > FS_IOC_SETFSLABEL ioctls. > > Eric Biggers (3): > f2fs: fix buffer overruns in FS_IOC_{GET,SET}FSLABEL > f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL > f2fs: add missing authorization check in FS_IOC_SETFSLABEL > > fs/f2fs/file.c | 27 +++++++++------------------ > 1 file changed, 9 insertions(+), 18 deletions(-) > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL 2019-08-16 6:49 ` [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Chao Yu @ 2019-08-18 13:03 ` Chao Yu 2019-08-18 15:42 ` Eric Biggers 0 siblings, 1 reply; 16+ messages in thread From: Chao Yu @ 2019-08-18 13:03 UTC (permalink / raw) To: Chao Yu, Eric Biggers, linux-f2fs-devel Hi Eric, Do you mind folding these fixes into original patch as it's still in dev branch? BTW, any comment or suggestion is welcome in my original patch.... :) Thanks, On 2019-8-16 14:49, Chao Yu wrote: > Eric, thanks for all the fixes. > > Thanks, > > On 2019/8/16 13:55, Eric Biggers wrote: >> Fix some bugs in the f2fs implementation of the FS_IOC_GETFSLABEL and >> FS_IOC_SETFSLABEL ioctls. >> >> Eric Biggers (3): >> f2fs: fix buffer overruns in FS_IOC_{GET,SET}FSLABEL >> f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL >> f2fs: add missing authorization check in FS_IOC_SETFSLABEL >> >> fs/f2fs/file.c | 27 +++++++++------------------ >> 1 file changed, 9 insertions(+), 18 deletions(-) >> > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL 2019-08-18 13:03 ` Chao Yu @ 2019-08-18 15:42 ` Eric Biggers 2019-08-19 3:03 ` Jaegeuk Kim 0 siblings, 1 reply; 16+ messages in thread From: Eric Biggers @ 2019-08-18 15:42 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel On Sun, Aug 18, 2019 at 09:03:21PM +0800, Chao Yu wrote: > Hi Eric, > > Do you mind folding these fixes into original patch as it's still in dev branch? > > BTW, any comment or suggestion is welcome in my original patch.... :) > > Thanks, > I'm fine with the patches being folded in. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL 2019-08-18 15:42 ` Eric Biggers @ 2019-08-19 3:03 ` Jaegeuk Kim 0 siblings, 0 replies; 16+ messages in thread From: Jaegeuk Kim @ 2019-08-19 3:03 UTC (permalink / raw) To: Chao Yu, Chao Yu, linux-f2fs-devel On 08/18, Eric Biggers wrote: > On Sun, Aug 18, 2019 at 09:03:21PM +0800, Chao Yu wrote: > > Hi Eric, > > > > Do you mind folding these fixes into original patch as it's still in dev branch? > > > > BTW, any comment or suggestion is welcome in my original patch.... :) > > > > Thanks, > > > > I'm fine with the patches being folded in. Okay, let me fold them. > > - Eric > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-08-19 3:24 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-16 5:55 [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Eric Biggers 2019-08-16 5:55 ` [f2fs-dev] [PATCH 1/3] f2fs: fix buffer overruns in FS_IOC_{GET, SET}FSLABEL Eric Biggers 2019-08-16 6:50 ` Chao Yu 2019-08-16 5:55 ` [f2fs-dev] [PATCH 2/3] f2fs: fix copying too many bytes in FS_IOC_SETFSLABEL Eric Biggers 2019-08-16 6:59 ` Chao Yu 2019-08-18 15:41 ` Eric Biggers 2019-08-19 1:33 ` Chao Yu 2019-08-19 1:58 ` Chao Yu 2019-08-19 2:55 ` Eric Biggers 2019-08-19 3:24 ` Chao Yu 2019-08-16 5:55 ` [f2fs-dev] [PATCH 3/3] f2fs: add missing authorization check " Eric Biggers 2019-08-16 7:00 ` Chao Yu 2019-08-16 6:49 ` [f2fs-dev] [PATCH 0/3] f2fs: fixes for FS_IOC_{GET,SET}FSLABEL Chao Yu 2019-08-18 13:03 ` Chao Yu 2019-08-18 15:42 ` Eric Biggers 2019-08-19 3:03 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).