Messages in this thread | | | Subject | Re: [PATCH v3 1/3] bpf: Add new bpf map type to store the pointer to struct perf_event | From | Alexei Starovoitov <> | Date | Thu, 23 Jul 2015 19:26:02 -0700 |
| |
On 7/23/15 7:22 PM, xiakaixu wrote: >>> + /* check if the value is already stored */ >>> >>+ if (array->events[index]) >>> >>+ return -EINVAL; >>> >>+ >>> >>+ /* convert the fd to the pointer to struct perf_event */ >>> >>+ event = convert_map_with_perf_event(value); >> > >> >imo helper name is misleading and it's too short to be separate >> >function. Just inline it and you can reuse 'index' variable. >> > >>> >>+ if (!event) >>> >>+ return -EBADF; >>> >>+ >>> >>+ xchg(array->events + index, event); >> > >> >refcnt leak of old event! Please think it through. >> >This type of bugs I shouldn't be finding. > Maybe the commit message is not elaborate. Here I prevent > user space from updating the existed event, so the return > value of xchg() is NULL and no refcnt leak of old event. > I will do the same as prog_array in next version.
I see then it's even worse. You think that above check: + if (array->events[index]) + return -EINVAL; will protect the double insert? It won't, since there are no locks here. You can have two processes both seeing empty slot and racing to do xchg.
| |