Re: kmem_alloc(0, f)
> Date: Tue, 1 Aug 2017 06:23:28 - (UTC) > From: mlel...@serpens.de (Michael van Elst) > > mar...@duskware.de (Martin Husemann) writes: > > >An empty array still is 99% of the times a driver bug and simply caught > >when first trying the driver. All other call sides can simply avoid the > >allocation and use a NULL pointer instead. > > Aren't zero sized allocations a must for Linux kernel shim compatibility? > If I remember correctly the major user was the radeon DRM code. Linux code for which we provide source compatibility uses Linux kmalloc, which allows zero-size allocations unlike kmem_alloc. (kmalloc uses malloc(9) currently; if adapted to use kmem(9), it would have to use nonzero-sized allocations anyway to store the size, since Linux kfree doesn't pass the size.) Anywhere we have adapted the code to use kmem_alloc, e.g. for a large chunk of NetBSD-specific code, has the same issue as any other driver: need to prove that the inputs can't be zero.
Re: kmem_alloc(0, f)
mar...@duskware.de (Martin Husemann) writes: >An empty array still is 99% of the times a driver bug and simply caught >when first trying the driver. All other call sides can simply avoid the >allocation and use a NULL pointer instead. Aren't zero sized allocations a must for Linux kernel shim compatibility? If I remember correctly the major user was the radeon DRM code. -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: kmem_alloc(0, f)
On Mon, Jul 31, 2017 at 10:13:28PM +, Taylor R Campbell wrote: > On reflection, perhaps the danger of kmem_alloc(0, f) would be > rendered moot by having a kmem array API that gracefully handles empty > arrays. An empty array still is 99% of the times a driver bug and simply caught when first trying the driver. All other call sides can simply avoid the allocation and use a NULL pointer instead. All drivers that do not properly check bounds on userland supplied ranges are a different story, but making the kmem* operation succeed will only paper over it and hide issues. Martin
Re: kmem_alloc(0, f)
> Date: Mon, 31 Jul 2017 09:55:22 +0200 > From: Joerg Sonnenberger > > On Sun, Jul 30, 2017 at 03:23:50PM -, Michael van Elst wrote: > > So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it > > cannot fail? I don't think that it can return a zero sized allocation > > (i.e. ptr != NULL that cannot be dereferenced). > > Just return a NULL pointer? That said, I do prefer just declaring it > invalid... On reflection, perhaps the danger of kmem_alloc(0, f) would be rendered moot by having a kmem array API that gracefully handles empty arrays. The length of an array is usually much more variable and more likely to be supplied by userland than the element size, so it's probably much less of a problem to panic on zero size -- chances are such bugs will be caught sooner, as long as we are careful to identify which parameter is size and which parameter is element count.
Re: kmem_alloc(0, f)
> Date: Sun, 30 Jul 2017 19:50:41 +0200 > From: Martin Husemann > > On Sun, Jul 30, 2017 at 03:23:50PM -, Michael van Elst wrote: > > So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it > > cannot fail? I don't think that it can return a zero sized allocation > > (i.e. ptr != NULL that cannot be dereferenced). > > Sure it could, return a pointer inside some red zone unmapped (but reserved > kva) page. On typical setups and modulo syscctl vm.user_va0_disable > e.g. "return (void*)16;" just as a simple example. It's slightly trickier than that because we expect that successful kmem_alloc yields distinct results each time. If we really wanted this, we could, say, use vmem(9) to allocate distinct bytes within these pages.
Re: kmem_alloc(0, f)
> Date: Sun, 30 Jul 2017 15:23:50 - (UTC) > From: mlel...@serpens.de (Michael van Elst) > > So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it > cannot fail? I don't think that it can return a zero sized allocation > (i.e. ptr != NULL that cannot be dereferenced). Does kmem_alloc(1, KM_SLEEP) always return a pointer p such that p + 1 and p - 1 cannot be dereferenced in practice? (Easy to arrange that for one of the two, but not both simultaneously.) We could just make kmem_alloc(0, f) always do the same as kmem_alloc(1, f), and likewise kmem_free(p, 0) -> kmem_free(p, 1). Could also put an 8-bit randomized hash of the address at p[0] on kmem_alloc and check it on kmem_free to raise the probability of detecting accidental writes there.
Re: kmem_alloc(0, f)
jo...@bec.de (Joerg Sonnenberger) writes: >On Sun, Jul 30, 2017 at 03:23:50PM -, Michael van Elst wrote: >> mar...@duskware.de (Martin Husemann) writes: >> >> >On Sat, Jul 29, 2017 at 02:04:42PM +, Taylor R Campbell wrote: >> >> This seems like a foot-oriented panic gun, and it's been a source of >> >> problems in the past. Can we change it? >> >> >I think it is a valuable tool to catch driver bugs early during >> >developement, but wouldn't mind to reduce it to a KASSERT. >> >> So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it >> cannot fail? I don't think that it can return a zero sized allocation >> (i.e. ptr != NULL that cannot be dereferenced). >Just return a NULL pointer? That said, I do prefer just declaring it >invalid... Returning a NULL pointer (currently equivalent to return failure) is what KM_SLEEP guarantees will not happen. By declaring a NULL pointer a failure if that flag was given (and a failure if that flag is missing) you open a can of worms. Declaring a zero sized allocation invalid (as is now) is indeed preferrable. -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: kmem_alloc(0, f)
On Sun, Jul 30, 2017 at 03:23:50PM -, Michael van Elst wrote: > mar...@duskware.de (Martin Husemann) writes: > > >On Sat, Jul 29, 2017 at 02:04:42PM +, Taylor R Campbell wrote: > >> This seems like a foot-oriented panic gun, and it's been a source of > >> problems in the past. Can we change it? > > >I think it is a valuable tool to catch driver bugs early during > >developement, but wouldn't mind to reduce it to a KASSERT. > > So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it > cannot fail? I don't think that it can return a zero sized allocation > (i.e. ptr != NULL that cannot be dereferenced). Just return a NULL pointer? That said, I do prefer just declaring it invalid... Joerg
Re: kmem_alloc(0, f)
On Sun, Jul 30, 2017 at 03:23:50PM -, Michael van Elst wrote: > So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it > cannot fail? I don't think that it can return a zero sized allocation > (i.e. ptr != NULL that cannot be dereferenced). Sure it could, return a pointer inside some red zone unmapped (but reserved kva) page. On typical setups and modulo syscctl vm.user_va0_disable e.g. "return (void*)16;" just as a simple example. Martin
Re: kmem_alloc(0, f)
mar...@duskware.de (Martin Husemann) writes: >On Sat, Jul 29, 2017 at 02:04:42PM +, Taylor R Campbell wrote: >> This seems like a foot-oriented panic gun, and it's been a source of >> problems in the past. Can we change it? >I think it is a valuable tool to catch driver bugs early during >developement, but wouldn't mind to reduce it to a KASSERT. So what does kmem_alloc(0, KM_SLEEP) do? fail where KM_SLEEP says it cannot fail? I don't think that it can return a zero sized allocation (i.e. ptr != NULL that cannot be dereferenced). -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: kmem_alloc(0, f)
On Sat, Jul 29, 2017 at 02:04:42PM +, Taylor R Campbell wrote: > This seems like a foot-oriented panic gun, and it's been a source of > problems in the past. Can we change it? I think it is a valuable tool to catch driver bugs early during developement, but wouldn't mind to reduce it to a KASSERT. Martin
kmem_alloc(0, f)
Why is kmem_alloc(0, f) a panic in NetBSD? It's not parroting Solaris -- in Solaris it's allowed, according to the man page. An assertion appears to have been added when ad@ implemented kmguard, but maybe there's logic deeper inside dependent on the restriction too. This seems like a foot-oriented panic gun, and it's been a source of problems in the past. Can we change it?
