Re: [PATCH v2] X.509: Introduce scope-based x509_certificate allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 12, 2024 at 11:07:06AM -0800, Dan Williams wrote:
> Lukas Wunner wrote:
> > In x509_cert_parse(), add a hint for the compiler that kzalloc() never
> > returns an ERR_PTR().  Otherwise the compiler adds a gratuitous IS_ERR()
> > check on return.  Introduce a handy assume() macro for this which can be
> > re-used elsewhere in the kernel to provide hints for the compiler.
[...]
> >  	cert = kzalloc(sizeof(struct x509_certificate), GFP_KERNEL);
> > +	assume(!IS_ERR(cert)); /* Avoid gratuitous IS_ERR() check on return */
> 
> I like the idea of assume() I just wonder if it should move inside of
> the kmalloc() inline definition itself? I.e. solve the "cleanup.h" vs
> ERR_PTR() rough edge more generally.

I've tried that but total vmlinux size increased by 448 bytes.
It seems to cause additional code or padding somewhere.  To avoid
pushback because of that, I confined it to just x509_cert_parse().

I should mention that there's a coccinelle rule which warns if
someone performs an IS_ERR() check on a kmalloc'ed pointer
(scripts/coccinelle/null/eno.cocci).  Which is why there likely
aren't any offenders in the tree.  That rule is triggered by
this assume() clause, but it's obviously a false-positive.
I'll look into suppressing that warning if/when this patch
gets accepted.

I should also mention that assume() currently only has an effect
with gcc.  clang-15 just ignored it during my testing.

Thanks,

Lukas




[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux