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
