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,

[PATCH 09/17] file: Implement fnext_task

2020-08-17 Thread Eric W. Biederman
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 file. This allows file descriptors of foreign processes to be iterated thro