On 10/17/2019 11:49 AM, Richard W.M. Jones wrote: > On Thu, Oct 17, 2019 at 09:36:34AM -0700, Eric Biggers wrote: >> On Thu, Oct 17, 2019 at 05:28:29PM +0100, Richard W.M. Jones wrote: >>> On Thu, Oct 17, 2019 at 10:47:59AM -0500, Mike Christie wrote: >>>> On 10/17/2019 09:03 AM, Richard W.M. Jones wrote: >>>>> On Tue, Oct 01, 2019 at 04:19:25PM -0500, Mike Christie wrote: >>>>>> Hey Josef and nbd list, >>>>>> >>>>>> I had a question about if there are any socket family restrictions for nbd? >>>>> >>>>> In normal circumstances, in userspace, the NBD protocol would only be >>>>> used over AF_UNIX or AF_INET/AF_INET6. >>>>> >>>>> There's a bit of confusion because netlink is used by nbd-client to >>>>> configure the NBD device, setting things like block size and timeouts >>>>> (instead of ioctl which is deprecated). I think you don't mean this >>>>> use of netlink? >>>> >>>> I didn't. It looks like it is just a bad test. >>>> >>>> For the automated test in this thread the test created a AF_NETLINK >>>> socket and passed it into the NBD_SET_SOCK ioctl. That is what got used >>>> for the NBD_DO_IT ioctl. >>>> >>>> I was not sure if the test creator picked any old socket and it just >>>> happened to pick one nbd never supported, or it was trying to simulate >>>> sockets that did not support the shutdown method. >>>> >>>> I attached the automated test that got run (test.c). >>> >>> I'd say it sounds like a bad test, but I'm not familiar with syzkaller >>> nor how / from where it generates these tests. Did someone report a >>> bug and then syzkaller wrote this test? >> >> It's an automatically generated fuzz test. >> >> There's rarely any such thing as a "bad" fuzz test. If userspace >> can do something that causes the kernel to crash or hang, it's a >> kernel bug, with very few exceptions (e.g. like writing to >> /dev/mem). >> >> If there are cases that aren't supported, like sockets that don't >> support a certain function or whatever, then the code needs to check >> for those cases and return an error, not hang the kernel. > > Oh I see. In that case I agree, although I believe this is a > root-only API and root has a lot of ways to crash the kernel, but sure > it could be fixed to restrict sockets to one of: > > - AF_LOCAL or AF_UNIX > - AF_INET or AF_INET6 > - AF_INET*_SDP (? no idea what this is, but it's used by nbd-client) > This one as for a infinniband related socket family that never made it upstream. It did support the shutdown callout, so I just made my patch check that the passed in socket support that instead of hard coding the family names just in case there was some user still using it.
