On Fri, Dec 22, 2017 at 8:59 AM, Craig Gallek <kraigatg...@gmail.com> wrote: > On Fri, Dec 22, 2017 at 3:11 AM, Nicolas Dichtel > <nicolas.dich...@6wind.com> wrote: >> Le 21/12/2017 à 23:18, Craig Gallek a écrit : >>> From: Craig Gallek <kr...@google.com> >>> >>> The below referenced commit extended the RTM_GETLINK interface to >>> allow querying by netns id. The netnsid property was previously >>> defined as a signed integer, but this patch assumes that the user >>> always passes a positive integer. syzkaller discovered this problem >>> by setting a negative netnsid and then calling the get-link path >>> in a tight loop. This surprisingly quickly overflows the reference >>> count on the associated struct net, potentially destroying it. When the >>> default namespace is used, the machine crashes in strange and interesting >>> ways. >>> >>> Unfortunately, this is not easy to reproduce with just the ip tool >>> as it enforces unsigned integer parsing despite the interface interpeting >>> the NETNSID attribute as signed. >>> >>> I'm not sure why this attribute is signed in the first place, but >>> the first commit that introduced it (6621dd29eb9b) is in v4.15-rc4, >>> so I assume it's too late to change. >> A valid (assigned) nsid is always >= 0. >> >>> >>> This patch removes the positive netns id assumption, but adds another >>> assumption that the netns id 0 is always the 'self' identifying id (for >>> which an additional struct net reference is not necessary). >> We cannot make this assumption, this is wrong. nsids may be automatically >> allocated by the kernel, and it starts by 0. >> The current netns can be identify by NETNSA_NSID_NOT_ASSIGNED, ie -1. > Thank you, I'll respin this with NETNSA_NSID_NOT_ASSIGNED as the sentinel > value.
Looking at the netns id code more closely, there are several places that assume ids will never be zero (short of the sentinel). I think the only simple fix here is to update the netlink interfaces to not accept negative values as input. I'm going to send that patch instead...
