On 3/27/2012 9:22 PM, Tetsuo Handa wrote: > security_inode_alloc() is called from inode_init_always(). > inode_init_always() is called from xfs_inode_alloc(). > > fs/xfs/xfs_iget.c: > 58 STATIC struct xfs_inode * > 59 xfs_inode_alloc( > 60 struct xfs_mount *mp, > 61 xfs_ino_t ino) > 62 { > 63 struct xfs_inode *ip; > 64 > 65 /* > 66 * if this didn't occur in transactions, we could use > 67 * KM_MAYFAIL and return NULL here on ENOMEM. Set the > 68 * code up to do this anyway. > 69 */ > 70 ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); > 71 if (!ip) > 72 return NULL; > 73 if (inode_init_always(mp->m_super, VFS_I(ip))) { > > kmem_flags_convert(KM_SLEEP) returns GFP_KERNEL or GFP_NOFS depending on > whether (current->flags & PF_FSTRANS) == 0 or not. > > fs/xfs/kmem.c: > 104 void * > 105 kmem_zone_alloc(kmem_zone_t *zone, unsigned int __nocast flags) > 106 { > 107 int retries = 0; > 108 gfp_t lflags = kmem_flags_convert(flags); > 109 void *ptr; > 110 > 111 do { > 112 ptr = kmem_cache_alloc(zone, lflags); > > fs/xfs/kmem.h: > 40 static inline gfp_t > 41 kmem_flags_convert(unsigned int __nocast flags) > 42 { > 43 gfp_t lflags; > 44 > 45 BUG_ON(flags & ~(KM_SLEEP|KM_NOSLEEP|KM_NOFS|KM_MAYFAIL)); > 46 > 47 if (flags & KM_NOSLEEP) { > 48 lflags = GFP_ATOMIC | __GFP_NOWARN; > 49 } else { > 50 lflags = GFP_KERNEL | __GFP_NOWARN; > 51 if ((current->flags & PF_FSTRANS) || (flags & KM_NOFS)) > 52 lflags &= ~__GFP_FS; > 53 } > 54 return lflags; > 55 } > > Therefore, should SMACK use GFP_NOFS like SELinux does? I am perfectly willing to make the change if it is currently incorrect. I will happily accept any expert advice this or any other Smack misuse of GFP flags. Thank you. > > security/smack/smack_lsm.c: > 520 static int smack_inode_alloc_security(struct inode *inode) > 521 { > 522 inode->i_security = new_inode_smack(smk_of_current()); > 523 if (inode->i_security == NULL) > 524 return -ENOMEM; > 525 return 0; > 526 } > > security/smack/smack_lsm.c: > 75 struct inode_smack *new_inode_smack(char *smack) > 76 { > 77 struct inode_smack *isp; > 78 > 79 isp = kzalloc(sizeof(struct inode_smack), GFP_KERNEL); > 80 if (isp == NULL) > 81 return NULL; > > security/selinux/hooks.c: > 199 static int inode_alloc_security(struct inode *inode) > 200 { > 201 struct inode_security_struct *isec; > 202 u32 sid = current_sid(); > 203 > 204 isec = kmem_cache_zalloc(sel_inode_cache, GFP_NOFS); > 205 if (!isec) > 206 return -ENOMEM; > > It would be nice if we could encapsulate downgrading from GFP_KERNEL to > GFP_NOFS into core of memory allocation functions that really need to check > whether GFP_FS is set or not, for > > (1) free page likely be found without checking whether GFP_FS is set or not > > (2) if GFP_KERNEL or GFP_NOFS is used, there must be a valid "struct > task_struct" which can remember the state whether I must use GFP_NOFS or > not (like XFS does). Maybe something like preemption level counter. > > (3) (I don't know whether below case can happen or not, but...:) > The upper filesystem layer starts something that causes current thread to > avoid use of GFP_FS, and the lower filesystem needs to do something that > entails memory allocation (e.g. read/write via network layer) caused by > request from upper filesystem layer. I'm worrying that the network layer > would fail to find that we are under the conditions where use of GFP_KERNEL > can cause problems to caller. > > If the SMACK question I mentioned above is true, there could be many locations > where GFP_KERNEL is errornously used. > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html