| Message ID | 1479121912-13384-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/14/2016 03:11 AM, Tetsuo Handa wrote: > Calling kmalloc(GFP_NOIO) with order == PAGE_ALLOC_COSTLY_ORDER is not > recommended because it might fall into infinite retry loop without > invoking the OOM killer. > > Since aa_dfa_unpack() is the only caller of kvzalloc() and > aa_dfa_unpack() which is calling kvzalloc() via unpack_table() is > doing kzalloc(GFP_KERNEL), it is safe to use GFP_KERNEL from > __aa_kvmalloc(). > > Since aa_simple_write_to_buffer() is the only caller of kvmalloc() > and aa_simple_write_to_buffer() is calling copy_from_user() which > is GFP_KERNEL context (see memdup_user_nul()), it is safe to use > GFP_KERNEL from __aa_kvmalloc(). > > Therefore, replace GFP_NOIO with GFP_KERNEL. Also, since we have > vmalloc() fallback, add __GFP_NORETRY so that we don't invoke the OOM > killer by kmalloc(GFP_KERNEL) with order == PAGE_ALLOC_COSTLY_ORDER. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > security/apparmor/lib.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c > index c1827e0..2ef422a 100644 > --- a/security/apparmor/lib.c > +++ b/security/apparmor/lib.c > @@ -95,7 +95,8 @@ void *__aa_kvmalloc(size_t size, gfp_t flags) > > /* do not attempt kmalloc if we need more than 16 pages at once */ > if (size <= (16*PAGE_SIZE)) > - buffer = kmalloc(size, flags | GFP_NOIO | __GFP_NOWARN); > + buffer = kmalloc(size, flags | GFP_KERNEL | __GFP_NORETRY | > + __GFP_NOWARN); > if (!buffer) { > if (flags & __GFP_ZERO) > buffer = vzalloc(size); > yep, thanks Tetsu I'll pull it into my tree and send it up Acked-by: John Johansen <john.johansen@canonical.com> -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index c1827e0..2ef422a 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -95,7 +95,8 @@ void *__aa_kvmalloc(size_t size, gfp_t flags) /* do not attempt kmalloc if we need more than 16 pages at once */ if (size <= (16*PAGE_SIZE)) - buffer = kmalloc(size, flags | GFP_NOIO | __GFP_NOWARN); + buffer = kmalloc(size, flags | GFP_KERNEL | __GFP_NORETRY | + __GFP_NOWARN); if (!buffer) { if (flags & __GFP_ZERO) buffer = vzalloc(size);
Calling kmalloc(GFP_NOIO) with order == PAGE_ALLOC_COSTLY_ORDER is not recommended because it might fall into infinite retry loop without invoking the OOM killer. Since aa_dfa_unpack() is the only caller of kvzalloc() and aa_dfa_unpack() which is calling kvzalloc() via unpack_table() is doing kzalloc(GFP_KERNEL), it is safe to use GFP_KERNEL from __aa_kvmalloc(). Since aa_simple_write_to_buffer() is the only caller of kvmalloc() and aa_simple_write_to_buffer() is calling copy_from_user() which is GFP_KERNEL context (see memdup_user_nul()), it is safe to use GFP_KERNEL from __aa_kvmalloc(). Therefore, replace GFP_NOIO with GFP_KERNEL. Also, since we have vmalloc() fallback, add __GFP_NORETRY so that we don't invoke the OOM killer by kmalloc(GFP_KERNEL) with order == PAGE_ALLOC_COSTLY_ORDER. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- security/apparmor/lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)