On Fri 14-12-18 11:55:52, Alexander Lochmann wrote: > > file_remove_privs() might be called for non-regular files, e.g. > blkdev inode. There is no reason to do its job on things > like blkdev inodes, pipes, or cdevs. Hence, abort if > file does not refer to a regular inode. > The following stacktrace shows how to get there: > 13: entry_SYSENTER_32:460 > 12: do_fast_syscall_32:410 > 11: _static_cpu_has:146 > 10: do_syscall_32_irqs_on:322 > 09: SyS_pwrite64:636 > 08: SYSC_pwrite64:650 > 07: fdput:38 > 06: vfs_write:560 > 05: __vfs_write:512 > 04: new_sync_write:500 > 03: blkdev_write_iter:1977 > 02: __generic_file_write_iter:2897 > 01: file_remove_privs:1818 > 00: inode_has_no_xattr:3163 > > Found by LockDoc (Alexander Lochmann, Horst Schirmeier and Olaf > Spinczyk) > > Signed-off-by: Alexander Lochmann <[email protected]> > Signed-off-by: Horst Schirmeier <[email protected]>
The patch looks good to me. You can add: Reviewed-by: Jan Kara <[email protected]> Honza > --- > fs/inode.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index 35d2108d567c..682088190413 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -1820,8 +1820,13 @@ int file_remove_privs(struct file *file) > int kill; > int error = 0; > > - /* Fast path for nothing security related */ > - if (IS_NOSEC(inode)) > + /* > + * Fast path for nothing security related. > + * As well for non-regular files, e.g. blkdev inodes. > + * For example, blkdev_write_iter() might get here > + * trying to remove privs which it is not allowed to. > + */ > + if (IS_NOSEC(inode) || !S_ISREG(inode->i_mode)) > return 0; > > kill = dentry_needs_remove_privs(dentry); > -- > 2.19.2 > -- Jan Kara <[email protected]> SUSE Labs, CR
