Re: [PATCH 5/5] types: Add standard __ob_trap and __ob_wrap scalar types

[Kees Cook]

On Wed, Apr 01, 2026 at 10:31:37AM +0200, Peter Zijlstra wrote:
> On Tue, Mar 31, 2026 at 01:31:16PM -0700, Kees Cook wrote:
> 
> (still slowly digesting the thread)
> 
> > Yeah, as you mentioned earlier, I'd agree that nesting is rarely
> > useful. The only thing I'd want to be careful about is ordering/scope. I
> > *think* it would just operate as a "goto" and things like the cleanup.h
> > handlers wouldn't be involved: they operate when a scope is crossed
> > like before. And I think the overflow result wouldn't be represented
> > anywhere. i.e. the wrapped/truncated value wouldn't be stored:
> > 
> > int func()
> > {
> > 	...
> > 	u8 __ob_trap product = 5;
> > 	...
> > 	product = a * b; // if store is truncated, goto __overflow
> > 	...
> > 	return product;
> > 
> > __overflow:
> > 	pr_info("%u\n", product); // shows "5"
> > 	return -1;
> > }
> 
> Note that there is a 'fun' problem with this in combination with
> cleanup.h.
> 
> Something like:
> 
> int func()
> {
> 	u8 __ob_trap prod = 0;
> 
> 	scoped_guard (mutex, &my_lock) {
> 		prod = a * b;
> 	}
> 
> 	return prod;
> 
> __overflow:
> 	// whatever
> 	return -1;
> }
> 
> is fine. *HOWEVER*, something like:
> 
> int func()
> {
> 	int __ob_trap size = base + count * extra;
> 	int err;
> 
> 	struct my_obj *obj __cleanup(kfree) = kzalloc(size, GFP_KERNEL);
> 
> 	err = my_obj_init(obj);
> 	if (err)
> 		return ERR_PTR(err);
> 
> 	return_ptr(obj);
> 
> __overflow:
> 	// what now..
> 	return NULL;
> }
> 
> is most terribly broken. Specifically, the goto will jump into the scope
> of obj -- and that is not allowed.

Right, this has been my primary concern about having an implicit "goto"
sprinkled basically anywhere into the code flow. However, it does seem
that initialization checking is aware of the problem:

void func(void)
{
        unsigned long __ob_trap value = ({ goto weird; 256; });
        size_t outcome = 99;

        outcome = get_outcome();

	pr_info("outcome: %zu\n", outcome);
        return;
weird:
        pr_info("value: %lu\n", value);
        pr_info("outcome: %zu\n", outcome);
}

../drivers/misc/lkdtm/bugs.c:1059:35: warning: variable 'outcome' is uninitialized when used here [-Wuninitialized]
 1059 |         pr_info("outcome: %zu\n", outcome);
      |                                   ^~~~~~~
../drivers/misc/lkdtm/bugs.c:1021:2: note: variable 'outcome' is declared here
 1021 |         size_t outcome = 99;
      |         ^
../drivers/misc/lkdtm/bugs.c:1058:33: warning: variable 'value' is uninitialized when used here [-Wuninitialized]
 1058 |         pr_info("value: %lu\n", value);
      |                                 ^~~~~
../drivers/misc/lkdtm/bugs.c:1020:2: note: variable 'value' is declared here
 1020 |         unsigned long __ob_trap value = ({ goto weird; 256; });
      |         ^

But most importantly, if I add a cleanup after it, it gets rejected:

        unsigned long __ob_trap value = ({ goto weird; 256; });
        size_t outcome = 99;
        u8 *obj __cleanup(kfree) = kzalloc(33, GFP_KERNEL);
	...

../drivers/misc/lkdtm/bugs.c:1021:37: error: cannot jump from this goto statement to its label
 1021 |         unsigned long __ob_trap value = ({ goto weird; 256; });
      |                                            ^
../drivers/misc/lkdtm/bugs.c:1023:6: note: jump bypasses initialization of variable with __attribute__((cleanup))
 1023 |         u8 *obj __cleanup(kfree) = kzalloc(33, GFP_KERNEL);
      |             ^


(Though I would note that GCC does _not_ refuse the jump when there is a
cleanup; it only see the other two uninitialized values.)

So that makes it not totally broken, but it does make it a bit fragile.



Another concern I have is dealing with older compilers and how to
"hide" the label and its code. e.g. if I remove the "goto" from above:

../drivers/misc/lkdtm/bugs.c:1060:1: warning: label 'weird' defined but not used [-Wunused-label]
 1060 | weird:
      | ^~~~~

Oddly, the unreachable code isn't a problem, so we could just wrap the
label is some macro like:

#define force_label(x) if (0) goto x; x

force_label(weird):
        pr_info("value: %lu\n", value);
        pr_info("outcome: %zu\n", outcome);


-- 
Kees Cook