lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Jul 2020 11:09:45 -0700 From: Nathan Chancellor <natechancellor@...il.com> To: Luca Stefani <luca.stefani.ge1@...il.com> Cc: akpm@...ux-foundation.org, freak07 <michalechner92@...glemail.com>, Anton Altaparmakov <anton@...era.com>, linux-ntfs-dev@...ts.sourceforge.net, linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com Subject: Re: [PATCH v2] ntfs: Fix ntfs_test_inode and ntfs_init_locked_inode function type On Sat, Jul 18, 2020 at 01:25:13PM +0200, Luca Stefani wrote: > Clang's Control Flow Integrity (CFI) is a security mechanism that can > help prevent JOP chains, deployed extensively in downstream kernels > used in Android. > > It's deployment is hindered by mismatches in function signatures. For > this case, we make callbacks match their intended function signature, > and cast parameters within them rather than casting the callback when > passed as a parameter. > > When running `mount -t ntfs ...` we observe the following trace: > > Call trace: > __cfi_check_fail+0x1c/0x24 > name_to_dev_t+0x0/0x404 > iget5_locked+0x594/0x5e8 > ntfs_fill_super+0xbfc/0x43ec > mount_bdev+0x30c/0x3cc > ntfs_mount+0x18/0x24 > mount_fs+0x1b0/0x380 > vfs_kern_mount+0x90/0x398 > do_mount+0x5d8/0x1a10 > SyS_mount+0x108/0x144 > el0_svc_naked+0x34/0x38 > > Signed-off-by: Luca Stefani <luca.stefani.ge1@...il.com> > Tested-by: freak07 <michalechner92@...glemail.com> > Acked-by: Anton Altaparmakov <anton@...era.com> Reviewed-by: Nathan Chancellor <natechancellor@...il.com> > --- > fs/ntfs/dir.c | 2 +- > fs/ntfs/inode.c | 27 ++++++++++++++------------- > fs/ntfs/inode.h | 4 +--- > fs/ntfs/mft.c | 4 ++-- > 4 files changed, 18 insertions(+), 19 deletions(-) > > diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c > index 3c4811469ae8..e278bfc5ee7f 100644 > --- a/fs/ntfs/dir.c > +++ b/fs/ntfs/dir.c > @@ -1503,7 +1503,7 @@ static int ntfs_dir_fsync(struct file *filp, loff_t start, loff_t end, > na.type = AT_BITMAP; > na.name = I30; > na.name_len = 4; > - bmp_vi = ilookup5(vi->i_sb, vi->i_ino, (test_t)ntfs_test_inode, &na); > + bmp_vi = ilookup5(vi->i_sb, vi->i_ino, ntfs_test_inode, &na); > if (bmp_vi) { > write_inode_now(bmp_vi, !datasync); > iput(bmp_vi); > diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c > index d4359a1df3d5..9bb9f0952b18 100644 > --- a/fs/ntfs/inode.c > +++ b/fs/ntfs/inode.c > @@ -30,10 +30,10 @@ > /** > * ntfs_test_inode - compare two (possibly fake) inodes for equality > * @vi: vfs inode which to test > - * @na: ntfs attribute which is being tested with > + * @data: data which is being tested with > * > * Compare the ntfs attribute embedded in the ntfs specific part of the vfs > - * inode @vi for equality with the ntfs attribute @na. > + * inode @vi for equality with the ntfs attribute @data. > * > * If searching for the normal file/directory inode, set @na->type to AT_UNUSED. > * @na->name and @na->name_len are then ignored. > @@ -43,8 +43,9 @@ > * NOTE: This function runs with the inode_hash_lock spin lock held so it is not > * allowed to sleep. > */ > -int ntfs_test_inode(struct inode *vi, ntfs_attr *na) > +int ntfs_test_inode(struct inode *vi, void *data) > { > + ntfs_attr *na = (ntfs_attr *)data; > ntfs_inode *ni; > > if (vi->i_ino != na->mft_no) > @@ -72,9 +73,9 @@ int ntfs_test_inode(struct inode *vi, ntfs_attr *na) > /** > * ntfs_init_locked_inode - initialize an inode > * @vi: vfs inode to initialize > - * @na: ntfs attribute which to initialize @vi to > + * @data: data which to initialize @vi to > * > - * Initialize the vfs inode @vi with the values from the ntfs attribute @na in > + * Initialize the vfs inode @vi with the values from the ntfs attribute @data in > * order to enable ntfs_test_inode() to do its work. > * > * If initializing the normal file/directory inode, set @na->type to AT_UNUSED. > @@ -87,8 +88,9 @@ int ntfs_test_inode(struct inode *vi, ntfs_attr *na) > * NOTE: This function runs with the inode->i_lock spin lock held so it is not > * allowed to sleep. (Hence the GFP_ATOMIC allocation.) > */ > -static int ntfs_init_locked_inode(struct inode *vi, ntfs_attr *na) > +static int ntfs_init_locked_inode(struct inode *vi, void *data) > { > + ntfs_attr *na = (ntfs_attr *)data; > ntfs_inode *ni = NTFS_I(vi); > > vi->i_ino = na->mft_no; > @@ -131,7 +133,6 @@ static int ntfs_init_locked_inode(struct inode *vi, ntfs_attr *na) > return 0; > } > > -typedef int (*set_t)(struct inode *, void *); > static int ntfs_read_locked_inode(struct inode *vi); > static int ntfs_read_locked_attr_inode(struct inode *base_vi, struct inode *vi); > static int ntfs_read_locked_index_inode(struct inode *base_vi, > @@ -164,8 +165,8 @@ struct inode *ntfs_iget(struct super_block *sb, unsigned long mft_no) > na.name = NULL; > na.name_len = 0; > > - vi = iget5_locked(sb, mft_no, (test_t)ntfs_test_inode, > - (set_t)ntfs_init_locked_inode, &na); > + vi = iget5_locked(sb, mft_no, ntfs_test_inode, > + ntfs_init_locked_inode, &na); > if (unlikely(!vi)) > return ERR_PTR(-ENOMEM); > > @@ -225,8 +226,8 @@ struct inode *ntfs_attr_iget(struct inode *base_vi, ATTR_TYPE type, > na.name = name; > na.name_len = name_len; > > - vi = iget5_locked(base_vi->i_sb, na.mft_no, (test_t)ntfs_test_inode, > - (set_t)ntfs_init_locked_inode, &na); > + vi = iget5_locked(base_vi->i_sb, na.mft_no, ntfs_test_inode, > + ntfs_init_locked_inode, &na); > if (unlikely(!vi)) > return ERR_PTR(-ENOMEM); > > @@ -280,8 +281,8 @@ struct inode *ntfs_index_iget(struct inode *base_vi, ntfschar *name, > na.name = name; > na.name_len = name_len; > > - vi = iget5_locked(base_vi->i_sb, na.mft_no, (test_t)ntfs_test_inode, > - (set_t)ntfs_init_locked_inode, &na); > + vi = iget5_locked(base_vi->i_sb, na.mft_no, ntfs_test_inode, > + ntfs_init_locked_inode, &na); > if (unlikely(!vi)) > return ERR_PTR(-ENOMEM); > > diff --git a/fs/ntfs/inode.h b/fs/ntfs/inode.h > index 98e670fbdd31..363e4e820673 100644 > --- a/fs/ntfs/inode.h > +++ b/fs/ntfs/inode.h > @@ -253,9 +253,7 @@ typedef struct { > ATTR_TYPE type; > } ntfs_attr; > > -typedef int (*test_t)(struct inode *, void *); > - > -extern int ntfs_test_inode(struct inode *vi, ntfs_attr *na); > +extern int ntfs_test_inode(struct inode *vi, void *data); > > extern struct inode *ntfs_iget(struct super_block *sb, unsigned long mft_no); > extern struct inode *ntfs_attr_iget(struct inode *base_vi, ATTR_TYPE type, > diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c > index fbb9f1bc623d..0d62cd5bb7f8 100644 > --- a/fs/ntfs/mft.c > +++ b/fs/ntfs/mft.c > @@ -958,7 +958,7 @@ bool ntfs_may_write_mft_record(ntfs_volume *vol, const unsigned long mft_no, > * dirty code path of the inode dirty code path when writing > * $MFT occurs. > */ > - vi = ilookup5_nowait(sb, mft_no, (test_t)ntfs_test_inode, &na); > + vi = ilookup5_nowait(sb, mft_no, ntfs_test_inode, &na); > } > if (vi) { > ntfs_debug("Base inode 0x%lx is in icache.", mft_no); > @@ -1019,7 +1019,7 @@ bool ntfs_may_write_mft_record(ntfs_volume *vol, const unsigned long mft_no, > vi = igrab(mft_vi); > BUG_ON(vi != mft_vi); > } else > - vi = ilookup5_nowait(sb, na.mft_no, (test_t)ntfs_test_inode, > + vi = ilookup5_nowait(sb, na.mft_no, ntfs_test_inode, > &na); > if (!vi) { > /* > -- > 2.27.0 >
Powered by blists - more mailing lists