Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <000000000000038dab0575476b73@google.com> <f3bcebc6-47a7-518e-70f7-c7e167621841@I-love.SAKURA.ne.jp>
In-Reply-To: <f3bcebc6-47a7-518e-70f7-c7e167621841@I-love.SAKURA.ne.jp>
From:   Paul Moore <paul@paul-moore.com>
Date:   Wed, 12 Sep 2018 23:02:02 -0400
Message-ID: <CAHC9VhT-Thu6KppC-MWzqkB7R1CaQA9DWXOQnG0b2uS9+rvzoA@mail.gmail.com>
Subject: Re: [PATCH] selinux: Add __GFP_NOWARN to allocation at str_read()
To:     penguin-kernel@i-love.sakura.ne.jp
Cc:     selinux@tycho.nsa.gov,
        syzbot+ac488b9811036cea7ea0@syzkaller.appspotmail.com,
        Eric Paris <eparis@parisplace.org>,
        linux-kernel@vger.kernel.org, peter.enderborg@sony.com,
        Stephen Smalley <sds@tycho.nsa.gov>,
        syzkaller-bugs@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 7, 2018 at 12:43 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> syzbot is hitting warning at str_read() [1] because len parameter can
> become larger than KMALLOC_MAX_SIZE. We don't need to emit warning for
> this case.
>
> [1] https://syzkaller.appspot.com/bug?id=7f2f5aad79ea8663c296a2eedb81978401a908f0
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Reported-by: syzbot <syzbot+ac488b9811036cea7ea0@syzkaller.appspotmail.com>
> ---
>  security/selinux/ss/policydb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index e9394e7..f4eadd3 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1101,7 +1101,7 @@ static int str_read(char **strp, gfp_t flags, void *fp, u32 len)
>         if ((len == 0) || (len == (u32)-1))
>                 return -EINVAL;
>
> -       str = kmalloc(len + 1, flags);
> +       str = kmalloc(len + 1, flags | __GFP_NOWARN);
>         if (!str)
>                 return -ENOMEM;

Thanks for the patch.

My eyes are starting to glaze over a bit chasing down all of the
different kmalloc() code paths trying to ensure that this always does
the right thing based on size of the allocation and the different slab
allocators ... are we sure that this will always return NULL when (len
+ 1) is greater than KMALLOC_MAX_SIZE for the different slab allocator
configurations?

-- 
paul moore
www.paul-moore.com