Christoph Hellwig <hch@xxxxxx> writes:
> On Sat, May 05, 2018 at 07:37:33AM -0500, Eric W. Biederman wrote:
>> Christoph Hellwig <hch@xxxxxx> writes:
>>
>> > The shole seq_file sequence already operates under a single RCU lock pair,
>> > so move the pid namespace lookup into it, and stop grabbing a reference
>> > and remove all kinds of boilerplate code.
>>
>> This is wrong.
>>
>> Move task_active_pid_ns(current) from open to seq_start actually means
>> that the results if you pass this proc file between callers the results
>> will change. So this breaks file descriptor passing.
>>
>> Open is a bad place to access current. In the middle of read/write is
>> broken.
>>
>>
>> In this particular instance looking up the pid namespace with
>> task_active_pid_ns was a personal brain fart. What the code should be
>> doing (with an appropriate helper) is:
>>
>> struct pid_namespace *pid_ns = inode->i_sb->s_fs_info;
>>
>> Because each mount of proc is bound to a pid namespace. Looking up the
>> pid namespace from the super_block is a much better way to go.
>
> What do you have in mind for the helper? For now I've thrown it in
> opencoded into my working tree, but I'd be glad to add a helper.
>
> struct pid_namespace *proc_pid_namespace(struct inode *inode)
> {
> // maybe warn on for s_magic not on procfs??
> return inode->i_sb->s_fs_info;
> }
That should work. Ideally out of line for the proc_fs.h version.
Basically it should be a cousin of PDE_DATA.
Eric
