netdev - Re: [PATCH] ipx: call ipxitf_put() in ioctl error path

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] [day] [month] [year] [list]

Message-ID: <CAGXu5j+sqS_LfuS5Jy2-AfYmRyKWhsyBo9bqHrnnyQ+ONA4nMA@mail.gmail.com>
Date:   Tue, 2 May 2017 16:42:33 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Dan Carpenter <dan.carpenter@...cle.com>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        "security@...nel.org" <security@...nel.org>,
        "secalert@...hat.com" <secalert@...hat.com>
Subject: Re: [PATCH] ipx: call ipxitf_put() in ioctl error path

On Tue, May 2, 2017 at 3:58 AM, Dan Carpenter <dan.carpenter@...cle.com> wrote:
> We should call ipxitf_put() if the copy_to_user() fails.
>
> Reported-by: 李强 <liqiang6-s@....cn>
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
>
> diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
> index 8a9219ff2e77..fa31ef29e3fa 100644
> --- a/net/ipx/af_ipx.c
> +++ b/net/ipx/af_ipx.c
> @@ -1168,11 +1168,10 @@ static int ipxitf_ioctl(unsigned int cmd, void __user *arg)
>                 sipx->sipx_network      = ipxif->if_netnum;
>                 memcpy(sipx->sipx_node, ipxif->if_node,
>                         sizeof(sipx->sipx_node));
> -               rc = -EFAULT;
> +               rc = 0;
>                 if (copy_to_user(arg, &ifr, sizeof(ifr)))
> -                       break;
> +                       rc = -EFAULT;
>                 ipxitf_put(ipxif);
> -               rc = 0;
>                 break;
>         }
>         case SIOCAIPXITFCRT:

This refcount overflow flaw (and resulting use-after-free) appears to
be reachable from unprivileged userspace, though I think it requires
an interface already be configured, so this is likely not much risk to
most users. Someone more familiar with IPX should double-check...

And, of course, I should point out this flaw would have been blocked
entirely by using refcount_t:
https://lkml.org/lkml/2017/3/17/193

And if it didn't require a configured interface, it would be mitigated
with module autoload blocking:
https://lkml.org/lkml/2017/4/19/1088

(Yes, yes, I know both are still being worked on, but this is a good
example to show another case where they'd have been useful.)

-Kees

-- 
Kees Cook
Pixel Security