Messages in this thread | | | Subject | Re: [RFC 1/4] net/ipv4/fib: Remove run-time check in tnode_alloc() | From | Dmitry Safonov <> | Date | Mon, 1 Apr 2019 16:55:02 +0100 |
| |
Hi Alexander,
On 4/1/19 4:40 PM, Alexander Duyck wrote: >> @@ -333,8 +328,7 @@ static struct tnode *tnode_alloc(int bits) >> { >> size_t size; >> >> - /* verify bits is within bounds */ >> - if (bits > TNODE_VMALLOC_MAX) >> + if ((BITS_PER_LONG <= KEYLENGTH) && unlikely(bits >= BITS_PER_LONG)) >> return NULL; >> >> /* determine size and verify it is non-zero and didn't overflow */ > > I think it would be better if we left TNODE_VMALLOC_MAX instead of > replacing it with BITS_PER_LONG. This way we know that we are limited > by the size of the node on 32b systems, and by the KEYLENGTH on 64b > systems. The basic idea is to maintain the logic as to why we are doing > it this way instead of just burying things by using built in constants > that are close enough to work. > > So for example I believe TNODE_VMALLOC_MAX is 31 on a 32b system.
This is also true after the change: bits == 31 will *not* return.
> The > main reason for that is because we have to subtract the TNODE_SIZE from > the upper limit for size. By replacing TNODE_VMALLOC_MAX with > BITS_PER_LONG that becomes abstracted away and it becomes more likely > that somebody will mishandle it later.
So, I wanted to remove run-time check here on x86_64.. I could do it by adding !CONFIG_64BIT around the check.
But, I thought about the value of the check: I believe it's here not to limit maximum allocated size: kzalloc()/vzalloc() will fail and we should be fine with that.
In my opinion it's rather to check that (1UL << bits) wouldn't result in UB.
Thanks, Dmitry
| |