On Tue, Feb 25, 2020 at 5:18 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Mon, Feb 24, 2020 at 04:35:00PM +0100, glider@xxxxxxxxxx wrote: > > Certain copy_from_user() invocations in binder.c are known to > > unconditionally initialize locals before their first use, like e.g. in > > the following case: > > > > struct binder_transaction_data tr; > > if (copy_from_user(&tr, ptr, sizeof(tr))) > > return -EFAULT; > > > > In such cases enabling CONFIG_INIT_STACK_ALL leads to insertion of > > redundant locals initialization that the compiler fails to remove. > > To work around this problem till Clang can deal with it, we apply > > __do_not_initialize to local Binder structures. > > It should be possible to write a Coccinelle script to identify all these > cases. (i.e. a single path from struct declaration to copy_from_user()) > and apply the changes automatically. This script could be checked into > scripts/coccinelle/ to help keep these markings in sync... The following script: ================================= @local_inited_by_cfu@ attribute name __no_initialize; identifier var; type T; statement stmt; @@ T var +__no_initialize ; if (copy_from_user(&var,..., sizeof(var))) stmt ================================= seems to do the job, but this transformation isn't idempotent: it inserts __no_initialize every time Coccinelle is invoked. It's hard to work around this problem, as attributes may only be parts of +-lines (i.e. I cannot write a rule that matches "T var __no_initialize") Linux kernel coccinelle scripts don't really contain good examples of adding attributes, and probably the spatch version used by most maintainers doesn't support them :( _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
