Re: Rust kernel policy

[Dan Carpenter]

On Thu, Feb 20, 2025 at 03:09:21PM +0100, Martin Uecker wrote:
> Am Donnerstag, dem 20.02.2025 um 16:46 +0300 schrieb Dan Carpenter:
> > On Thu, Feb 20, 2025 at 09:57:29AM +0100, Martin Uecker wrote:
> > > In particulary, I have a relatively concrete plan to have a memory safe
> > > mode for C that can be toggled for some region of code and would make
> > > sure there is no UB or memory safety issues left (I am experimenting with
> > > this in the GCC FE).  So the idea is that one could start to activate this
> > > for certain critical regions of code to make sure there is no signed
> > > integer overflow or OOB access in it.
> > 
> > I don't think diferentiating between signed and unsigned integer
> > overflows is useful.  In the kernel, most security issues from integer
> > overflows are from unsigned integer overflows.  Kees says that we
> > should warn about "Unexpected" behavior instead of "Undefined".  In fact,
> > Justin Stitt has done the opposite of what you're doing and only checks
> > for unsigned overflows.  He created a sanitizer that warns about integer
> > overflows involving size_t type (which is unsigned), because sizes are
> > so important.  (Checking only size_t avoids probably the largest source
> > of harmless integer overflows which is dealing with time).
> 
> I agree with you.  We were also discussing an attribute that
> can be attached to certain unsigned types to indicate that
> wrapping is an error. 
> 
> My more limited aim (because my personal time is very limited)
> is to define a memory safe subset and in such a subset you can
> not have UB.  Hence, I am more focussed on signed overflow at
> the moment, but I agree that safety in general must go beyond 
> this.
> 
> But this is why I want the kernel community to be more involved,
> to get more resources and more experience into these discussions.
> 

In the kernel we use the -fwrapv so signed overflow is defined.
I used to have a static checker warning for signed integer
overflow.  There weren't many warnings but everything I looked at
ended up being safe because of -fwrapv so I disabled it.

(This was some time ago so my memory is vague).

> > 
> > The sanitizer has a list of exceptions like if (a < a + b) where the
> > integer overflow is idiomatic.  But the concern was that there might be
> > other deliberate integer overflows which aren't in the exception list so
> > Justin also created a macro to turn off the santizer.
> > 
> > 	x = wrapping_ok(a + b);
> 
> Indeed. This is the main issue with unsigned wraparound. Exactly
> because it was always defined, simply screening for wraparound
> yields many false positives. 
> 
> (BTW: Rust is also not perfectly immune to such errors:
> https://rustsec.org/advisories/RUSTSEC-2023-0080.html)
> 
> 
> > 
> > What I would like is a similar macro so we could write code like:
> > 
> > 	x = saturate_math(a + b + c + d * d_size);
> > 
> > If anything overflowed the result would be ULONG_MAX.  In the kernel,
> > we have the size_add() and size_mul() macros which do saturation math
> > instead of wrapping math but we'd have to say:
> > 
> > 	x = size_add(a, size_add(b, size_add(c, size_add(size_mul(d, d_size)))));
> > 
> > Which is super ugly.  Maybe we could create something like this macro?
> > 
> > #define saturate_math(x) ({             \
> >         unsigned long res;              \
> >         __trap_overflow(label_name));   \
> >         res = (x);                      \
> >         if (0) {                        \
> > lable_name:                             \
> >                 res = ULONG_MAX;        \
> >         }                               \
> >         res;                            \
> > })
> > 
> 
> We added checked arhithmetic to C23, we could add saturating
> math to C2Y if this is needed.  (although I admit I do not fully
> understand the use case of saturating math, a saturated value
> still seems to be an error? Statistics, where it does not matter?)
> 

Normally, you pass the resulting size to kmalloc() and kmalloc()
can't allocate ULONG_MAX bytes so the allocation fails harmlessly.
Where with an integer overflow, you do:

	buf = kmalloc(nr * size, GFP_KERNEL);
	if (!buf)
		return -ENOMEM;

	for (i = 0; i < nr; i++) {
		buf[i] = x;  <-- memory corruption

The buf is smaller than intended and it results in memory
corruption.

> In general, if people have good ideas what compilers or the language
> standard can do to help, please talk to us. It is possible to
> improve compilers and/or the language itself.

Thanks so much!

regards,
dan carpenter