Re: [patch v4 resend 2/2] kcmp: Add KCMP_EPOLL_TFD mode to compare epoll target files

On Fri, May 12, 2017 at 06:45:09PM -0700, Andrei Vagin wrote:

On Sat, May 13, 2017 at 01:53:40AM +0300, Cyrill Gorcunov wrote:

quoted

On Sat, May 13, 2017 at 12:41:30AM +0200, Jann Horn wrote:

quoted

[resending as plaintext]

I realize that the existing kcmp code has the same issue, but:

Why are you not taking a reference to filp or filp_tgt? This can end up
performing a comparison between a pointer to a freed struct file and a
pointer to a struct file that was allocated afterwards, right? So it can
return a false "is equal" result when the two files aren't actually the same
if one of the target tasks is running? This looks like it unnecessarily
exposes information about whether an allocation reuses the memory of
a previously freed allocation.

It work with unlocked data on purpose for speed sake. Moreover even
if we grap a reference it is valid _only_ during comparision operation,
next we drop ref and it can be easily freed by os. Thus it's up to
a caller to keep references to files/task and other resources used.

Looks like we can take rcu_read_lock() to guarantee that these objects
will not be freed, and rcu_read_lock() should not affect perfomance too much.

Rather they should be get_file_rcu/fput. Still I'm not convinced we need it,
but fine will update both: plain KCMP_FILE and KCMP_EPOLL_TFD since it won't
hurt performance.