Re: [PATCH 09/17] file: Implement fnext_task

2020-08-21 Thread Alexei Starovoitov
On Fri, Aug 21, 2020 at 8:26 AM Eric W. Biederman wrote: > > Alexei Starovoitov writes: > > > On Wed, Aug 19, 2020 at 6:25 AM Eric W. Biederman > > wrote: > >> > >> The bug in the existing code is that bpf_iter does get_file instead > >> of get_file_rcu. Does anyone have any sense of how to ad

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-20 Thread Cyrill Gorcunov
On Mon, Aug 17, 2020 at 05:04:17PM -0500, Eric W. Biederman wrote: > As a companion to fget_task and fcheck_task implement fnext_task that > will return the struct file for the first file descriptor show number > is equal or greater than the fd argument value, or NULL if there is > no such struct f

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-19 Thread Linus Torvalds
On Wed, Aug 19, 2020 at 6:25 AM Eric W. Biederman wrote: > > So I sat down and played with it and here is what I wound up with is: > > __fcheck_files -> files_lookup_fd_raw > fcheck_files -> files_lookup_fd_locked > fcheck_files -> files_lookup_fd_rcu > fcheck -> lookup_fd_rcu > ... >

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-19 Thread Alexei Starovoitov
On Wed, Aug 19, 2020 at 6:25 AM Eric W. Biederman wrote: > > The bug in the existing code is that bpf_iter does get_file instead > of get_file_rcu. Does anyone have any sense of how to add debugging > to get_file to notice when it is being called in the wrong context? That bug is already fixed i

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-18 Thread Christian Brauner
On Mon, Aug 17, 2020 at 06:17:35PM -0700, Linus Torvalds wrote: > On Mon, Aug 17, 2020 at 6:06 PM Eric W. Biederman > wrote: > > > > I struggle with the fcheck name as I have not seen or at least not > > registed on the the user that just checks to see if the result is NULL. > > So the name fchec

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-17 Thread Linus Torvalds
On Mon, Aug 17, 2020 at 6:06 PM Eric W. Biederman wrote: > > I struggle with the fcheck name as I have not seen or at least not > registed on the the user that just checks to see if the result is NULL. > So the name fcheck never made a bit of sense to me. Yeah, that name is not great. I just don'

Re: [PATCH 09/17] file: Implement fnext_task

2020-08-17 Thread Linus Torvalds
I like the series, but I have to say that the extension of the horrible "fcheck*()" interfaces raises my hackles.. That interface is very very questionable to begin with, and it's easy to get wrong. I don't see you getting it wrong - you do seem to hold the RCU read lock in the places I checked,