Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-18 Thread Oleg Nesterov
On 03/17, Oleg Nesterov wrote: > > Ah, I didn't mean the patch makes sense because of optimization. My > point was, we can fix the race without making this code worse (in fact > it tries to make the code better but this is subjective). OK, I am stupid. argv_free() passes the wrong poiinter to kfr

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-17 Thread Oleg Nesterov
On 03/16, Andi Kleen wrote: > > On Sat, Mar 16, 2013 at 10:23:51PM +0100, Oleg Nesterov wrote: > > > > > > rcu strings has a helper function to copy the string for sleepy cases. > > > > Then you need to pre-allocate, take rcu_read_lock(), copy, and check > > that it actually fits the pre-allocated

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Andi Kleen
On Sat, Mar 16, 2013 at 10:23:51PM +0100, Oleg Nesterov wrote: > On 03/16, Andi Kleen wrote: > > > > > Perhaps rcu can be better, although a global rwsem looks simpler, > > > I dunno. > > > > It's a general problem with lots of sysctls. > > > > > > But argv_split() or its usage should be changed an

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Oleg Nesterov
On 03/16, Andi Kleen wrote: > > > Perhaps rcu can be better, although a global rwsem looks simpler, > > I dunno. > > It's a general problem with lots of sysctls. > > > > But argv_split() or its usage should be changed anyway, and GFP_KERNEL > > won't work under rcu_read_lock(). > > rcu strings has

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Andi Kleen
> Perhaps rcu can be better, although a global rwsem looks simpler, > I dunno. It's a general problem with lots of sysctls. > > But argv_split() or its usage should be changed anyway, and GFP_KERNEL > won't work under rcu_read_lock(). rcu strings has a helper function to copy the string for slee

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Oleg Nesterov
On 03/16, Andi Kleen wrote: > > On Sat, Mar 16, 2013 at 09:23:27PM +0100, Oleg Nesterov wrote: > > On 03/15, Oleg Nesterov wrote: > > > > > > To remind, say, argv_split(poweroff_cmd) can race with sysctl changing > > > this > > > string, in this case it can write to the memory after argv[] array.

Re: [PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Andi Kleen
On Sat, Mar 16, 2013 at 09:23:27PM +0100, Oleg Nesterov wrote: > On 03/15, Oleg Nesterov wrote: > > > > To remind, say, argv_split(poweroff_cmd) can race with sysctl changing this > > string, in this case it can write to the memory after argv[] array. We can > > fix this, or we can rewrite argv_spl

[PATCH 0/2] finx argv_split() vs sysctl race

2013-03-16 Thread Oleg Nesterov
On 03/15, Oleg Nesterov wrote: > > To remind, say, argv_split(poweroff_cmd) can race with sysctl changing this > string, in this case it can write to the memory after argv[] array. We can > fix this, or we can rewrite argv_split/free: OK, please see 1/2. And this reminds me about set_task_comm()