linux-kernel - Re: [GIT PULL] Smack patches for v5.4

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wg1zkUTdnx5pNVOf=uuSJiEywNiztQe4oRiPb1pfA399w@mail.gmail.com>
Date:   Mon, 23 Sep 2019 14:35:28 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Casey Schaufler <casey@...aufler-ca.com>
Cc:     Linux Security Module list 
        <linux-security-module@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] Smack patches for v5.4 - retry

On Mon, Sep 23, 2019 at 1:14 PM Casey Schaufler <casey@...aufler-ca.com> wrote:
>
> Thank for the instruction. I think this is correct.

Looks fine, pulled.

That said, when I look closer:

> Jia-Ju Bai (1):
>       security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()

This one seems wrong.

Not seriously so, but the quoting the logic from the commit:

    In smack_socket_sock_rcv_skb(), there is an if statement
    on line 3920 to check whether skb is NULL:

        if (skb && skb->secmark != 0)

    This check indicates skb can be NULL in some cases.

and the fact is, skb _cannot_ be NULL, because when you test the
security of receiving an skb, you by definition always have an skb.

There is one single place that calls security_sock_rcv_skb(), and it
very much has a real skb.

So instead of adding a _new_ test for skb being NULL, the old test for
a NULL skb should just have been removed. It really doesn't make any
sense to have a NULL skb in that path - if some memory allocation had
failed on the receive path, that just means that the receive is never
done, it doesn't mean that you'd test a NULL skb for security policy
violations.

Anyway, it's pulled, but I think somebody should have checked and
thought about the automated tool reports a bit more..

               Linus