* [PATCH] ipx: call ipxitf_put() in ioctl error path [not found] <143C0AFC63FC204CB0C55BB88F3A8ABB33419ED1@EX02.corp.qihoo.net> @ 2017-05-02 10:58 ` Dan Carpenter 2017-05-02 19:35 ` David Miller 2017-05-02 23:42 ` [kernel-hardening] " Kees Cook 0 siblings, 2 replies; 4+ messages in thread From: Dan Carpenter @ 2017-05-02 10:58 UTC (permalink / raw) To: David S. Miller; +Cc: netdev, security, secalert We should call ipxitf_put() if the copy_to_user() fails. Reported-by: 李强 <liqiang6-s@360.cn> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.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: ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ipx: call ipxitf_put() in ioctl error path 2017-05-02 10:58 ` [PATCH] ipx: call ipxitf_put() in ioctl error path Dan Carpenter @ 2017-05-02 19:35 ` David Miller 2017-05-02 23:42 ` [kernel-hardening] " Kees Cook 1 sibling, 0 replies; 4+ messages in thread From: David Miller @ 2017-05-02 19:35 UTC (permalink / raw) To: dan.carpenter; +Cc: netdev, security, secalert From: Dan Carpenter <dan.carpenter@oracle.com> Date: Tue, 2 May 2017 13:58:53 +0300 > We should call ipxitf_put() if the copy_to_user() fails. > > Reported-by: 李强 <liqiang6-s@360.cn> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, thanks Dan. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ipx: call ipxitf_put() in ioctl error path 2017-05-02 10:58 ` [PATCH] ipx: call ipxitf_put() in ioctl error path Dan Carpenter @ 2017-05-02 23:42 ` Kees Cook 2017-05-02 23:42 ` [kernel-hardening] " Kees Cook 1 sibling, 0 replies; 4+ messages in thread From: Kees Cook @ 2017-05-02 23:42 UTC (permalink / raw) To: Dan Carpenter, kernel-hardening Cc: David S. Miller, Network Development, security, secalert On Tue, May 2, 2017 at 3:58 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > We should call ipxitf_put() if the copy_to_user() fails. > > Reported-by: 李强 <liqiang6-s@360.cn> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [kernel-hardening] Re: [PATCH] ipx: call ipxitf_put() in ioctl error path @ 2017-05-02 23:42 ` Kees Cook 0 siblings, 0 replies; 4+ messages in thread From: Kees Cook @ 2017-05-02 23:42 UTC (permalink / raw) To: Dan Carpenter, kernel-hardening Cc: David S. Miller, Network Development, security, secalert On Tue, May 2, 2017 at 3:58 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > We should call ipxitf_put() if the copy_to_user() fails. > > Reported-by: 李强 <liqiang6-s@360.cn> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-02 23:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <143C0AFC63FC204CB0C55BB88F3A8ABB33419ED1@EX02.corp.qihoo.net>
2017-05-02 10:58 ` [PATCH] ipx: call ipxitf_put() in ioctl error path Dan Carpenter
2017-05-02 19:35 ` David Miller
2017-05-02 23:42 ` Kees Cook
2017-05-02 23:42 ` [kernel-hardening] " Kees Cook
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.