On Tue, Feb 25, 2020 at 4:24 PM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > 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") This one: ============================ @match@ type T; identifier var; statement stmt; fresh identifier var_noinit = var##" __no_initialize"; @@ -T var; +T var_noinit; ... if (copy_from_user(&var,..., sizeof(var))) stmt ============================ is better, because it: - uses a "fresh identifier" hack instead of dealing with attributes (which aren't supported by spatch 1.0.4) - seems to be idempotent because of that hack. I'll regenerate the binder patch using that script and will look into enhancing it and committing it to scripts/coccinelle. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
