Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 4:37 PM, Christian Brauner wrote: > > In fact, /dev/ptmx being a symlink or bind-mount is the *standard* in > containers > even for non-user namespaced containers or containers that do not retain > CAP_MKNOD. Yes. I think using

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 4:37 PM, Christian Brauner wrote: > > In fact, /dev/ptmx being a symlink or bind-mount is the *standard* in > containers > even for non-user namespaced containers or containers that do not retain > CAP_MKNOD. Yes. I think using /dev/pts/ptmx is nice from a kernel

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 06:01:36PM -0500, Eric W. Biederman wrote: > Linus Torvalds writes: > > > On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman > > wrote: > >> > >> There are just enough weird one off scripts like xen image builder (I >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 06:01:36PM -0500, Eric W. Biederman wrote: > Linus Torvalds writes: > > > On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman > > wrote: > >> > >> There are just enough weird one off scripts like xen image builder (I > >> think that was the nasty test case that broke in

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 4:01 PM, Eric W. Biederman wrote: > Linus Torvalds writes: > >> On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman >> wrote: >>> >>> There are just enough weird one off scripts like xen image

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 4:01 PM, Eric W. Biederman wrote: > Linus Torvalds writes: > >> On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman >> wrote: >>> >>> There are just enough weird one off scripts like xen image builder (I >>> think that was the nasty test case that broke in debian) that I

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman > wrote: >> >> There are just enough weird one off scripts like xen image builder (I >> think that was the nasty test case that broke in debian) that I can't >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman > wrote: >> >> There are just enough weird one off scripts like xen image builder (I >> think that was the nasty test case that broke in debian) that I can't >> imagine ever being able to responsibly remove the path

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman wrote: > > There are just enough weird one off scripts like xen image builder (I > think that was the nasty test case that broke in debian) that I can't > imagine ever being able to responsibly remove the path based lookups

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 1:43 PM, Eric W. Biederman wrote: > > There are just enough weird one off scripts like xen image builder (I > think that was the nasty test case that broke in debian) that I can't > imagine ever being able to responsibly remove the path based lookups in > /dev/ptmx. I do

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 12:01 PM, Christian Brauner > wrote: >> >> I've touched on this in my original message, I wonder whether we currently >> support mounting devpts at a different a location and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 12:01 PM, Christian Brauner > wrote: >> >> I've touched on this in my original message, I wonder whether we currently >> support mounting devpts at a different a location and expect an open on a >> newly created slave to work. > > Yes. That is

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 1:24 PM, Stefan Lippers-Hollmann wrote: > > This patch seems to work, ssh, xterm (konsole5), real tty and pbuilder > (creating- and updating the build chroots, just as well as building > several fairly involved packages) are fine with this patch on top of >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 1:24 PM, Stefan Lippers-Hollmann wrote: > > This patch seems to work, ssh, xterm (konsole5), real tty and pbuilder > (creating- and updating the build chroots, just as well as building > several fairly involved packages) are fine with this patch on top of >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi [ sorry for the re-send, this accidentally only reached you, rather than the mailing list and the other recipients as well ] On 2017-08-24, Linus Torvalds wrote: > On Thu, Aug 24, 2017 at 11:31 AM, Linus Torvalds > wrote: > > > > It should just do a "return

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi [ sorry for the re-send, this accidentally only reached you, rather than the mailing list and the other recipients as well ] On 2017-08-24, Linus Torvalds wrote: > On Thu, Aug 24, 2017 at 11:31 AM, Linus Torvalds > wrote: > > > > It should just do a "return ptm_open_peer(file, tty,

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Christian Brauner writes: > On Aug 24, 2017 20:41, "Eric W. Biederman" wrote: > > > The implementation of TIOCGPTPEER has two issues. > > > > When /dev/ptmx (as opposed to > > I've touched on this in my original message, I wonder whether

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Christian Brauner writes: > On Aug 24, 2017 20:41, "Eric W. Biederman" wrote: > > > The implementation of TIOCGPTPEER has two issues. > > > > When /dev/ptmx (as opposed to > > I've touched on this in my original message, I wonder whether we > currently support mounting devpts at a different a

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 12:22 PM, Linus Torvalds wrote: > [root@i7 dummy]# ../a.out > I point to "/home/torvalds/dummy/pts/0" Note: that "a.out" binary is modified from your original code. It's modified to correctly print out the readdir() results, but it's

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 12:22 PM, Linus Torvalds wrote: > [root@i7 dummy]# ../a.out > I point to "/home/torvalds/dummy/pts/0" Note: that "a.out" binary is modified from your original code. It's modified to correctly print out the readdir() results, but it's also modified to open just

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 11:40 AM, Eric W. Biederman > wrote: >> >> Here is my tested version of the patch. > > Can you please take my cleanups to devpts_ptmx_path() too? Let met take a look. > Those 'goto err'

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 11:40 AM, Eric W. Biederman > wrote: >> >> Here is my tested version of the patch. > > Can you please take my cleanups to devpts_ptmx_path() too? Let met take a look. > Those 'goto err' statements are disgusting, when a plain 'return >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 12:01 PM, Christian Brauner wrote: > > I've touched on this in my original message, I wonder whether we currently > support mounting devpts at a different a location and expect an open on a > newly created slave to work. Yes. That is

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 12:01 PM, Christian Brauner wrote: > > I've touched on this in my original message, I wonder whether we currently > support mounting devpts at a different a location and expect an open on a > newly created slave to work. Yes. That is very much intended to work. > Say

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:40 AM, Eric W. Biederman wrote: > > Here is my tested version of the patch. Can you please take my cleanups to devpts_ptmx_path() too? Those 'goto err' statements are disgusting, when a plain 'return -ERRNO' works cleaner. And that "struct file

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:40 AM, Eric W. Biederman wrote: > > Here is my tested version of the patch. Can you please take my cleanups to devpts_ptmx_path() too? Those 'goto err' statements are disgusting, when a plain 'return -ERRNO' works cleaner. And that "struct file *filp = NULL;" is

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 11:06 AM, Linus Torvalds > wrote: >> >> But that still fails the TIOCGPTPEER ioctl with an NULL pointer >> dereference in devpts_mnt, I probably messed up when I fixed the >> dentry

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Thu, Aug 24, 2017 at 11:06 AM, Linus Torvalds > wrote: >> >> But that still fails the TIOCGPTPEER ioctl with an NULL pointer >> dereference in devpts_mnt, I probably messed up when I fixed the >> dentry refcount leak. > > No, that was another bug in the original

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:31 AM, Linus Torvalds wrote: > > It should just do a "return ptm_open_peer(file, tty, (int)arg);" instead. Here's the actual tested patch. It "WorksForMe(tm)", including the TIOCGPTPEER ioctl, and also verified that it gets the pathname

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:31 AM, Linus Torvalds wrote: > > It should just do a "return ptm_open_peer(file, tty, (int)arg);" instead. Here's the actual tested patch. It "WorksForMe(tm)", including the TIOCGPTPEER ioctl, and also verified that it gets the pathname right in /proc, which was the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:13 AM, Linus Torvalds wrote: > > The attached patch should work. It's Eric's original patch with > various cleanups and fixes. No, one more bug in there: Eric did + case TIOCGPTPEER: +retval = ptm_open_peer(file, tty, (int)arg);

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:13 AM, Linus Torvalds wrote: > > The attached patch should work. It's Eric's original patch with > various cleanups and fixes. No, one more bug in there: Eric did + case TIOCGPTPEER: +retval = ptm_open_peer(file, tty, (int)arg); +break; to call that

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:06 AM, Linus Torvalds wrote: > > But that still fails the TIOCGPTPEER ioctl with an NULL pointer > dereference in devpts_mnt, I probably messed up when I fixed the > dentry refcount leak. No, that was another bug in the original patch.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 11:06 AM, Linus Torvalds wrote: > > But that still fails the TIOCGPTPEER ioctl with an NULL pointer > dereference in devpts_mnt, I probably messed up when I fixed the > dentry refcount leak. No, that was another bug in the original patch. ptm_open_peer() passed in

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 10:52 AM, Linus Torvalds wrote: > > I'll test the fix. Yes, that was it, and things work with that fixed. But that still fails the TIOCGPTPEER ioctl with an NULL pointer dereference in devpts_mnt, I probably messed up when I fixed the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 10:52 AM, Linus Torvalds wrote: > > I'll test the fix. Yes, that was it, and things work with that fixed. But that still fails the TIOCGPTPEER ioctl with an NULL pointer dereference in devpts_mnt, I probably messed up when I fixed the dentry refcount leak.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 8:54 AM, Eric W. Biederman wrote: > > Weird. There is at least one leak inducing bug in there. So perhaps > that is the cause. *Scratches my head* Are you also testing the new > ioctl? I can verify, and it's not the leak. I tried your patch

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Thu, Aug 24, 2017 at 8:54 AM, Eric W. Biederman wrote: > > Weird. There is at least one leak inducing bug in there. So perhaps > that is the cause. *Scratches my head* Are you also testing the new > ioctl? I can verify, and it's not the leak. I tried your patch with that leak fix (and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Stefan Lippers-Hollmann writes: > Hi > > On 2017-08-23, Eric W. Biederman wrote: >> Linus Torvalds writes: >> > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds >> > wrote: > [...] >> This is so far untested (except

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Stefan Lippers-Hollmann writes: > Hi > > On 2017-08-23, Eric W. Biederman wrote: >> Linus Torvalds writes: >> > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds >> > wrote: > [...] >> This is so far untested (except for compiling) but I think this will >> work. >> >> I factor out

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 8:11 PM, Eric W. Biederman > wrote: >> -static int pty_get_peer(struct tty_struct *tty, int flags) >> +int ptm_open_peer(struct file *master, struct tty_struct *tty, int flags) >> { >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 8:11 PM, Eric W. Biederman > wrote: >> -static int pty_get_peer(struct tty_struct *tty, int flags) >> +int ptm_open_peer(struct file *master, struct tty_struct *tty, int flags) >> { >> int fd = -1; >> struct file *filp = NULL; >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi On 2017-08-23, Eric W. Biederman wrote: > Linus Torvalds writes: > > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds > > wrote: [...] > This is so far untested (except for compiling) but I think this will > work. > > I factor

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi On 2017-08-23, Eric W. Biederman wrote: > Linus Torvalds writes: > > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds > > wrote: [...] > This is so far untested (except for compiling) but I think this will > work. > > I factor out devpts_ptmx_path out of devpts_acquire so the code >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 8:11 PM, Eric W. Biederman wrote: > -static int pty_get_peer(struct tty_struct *tty, int flags) > +int ptm_open_peer(struct file *master, struct tty_struct *tty, int flags) > { > int fd = -1; > struct file *filp = NULL; > int

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 8:11 PM, Eric W. Biederman wrote: > -static int pty_get_peer(struct tty_struct *tty, int flags) > +int ptm_open_peer(struct file *master, struct tty_struct *tty, int flags) > { > int fd = -1; > struct file *filp = NULL; > int retval = -EINVAL; > +

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds > wrote: >> >> Argh. And it's *not* fairly straightforward, because the >> tty_operations "ioctl()" function pointer only gets 'struct tty *'. >> >> So in the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds > wrote: >> >> Argh. And it's *not* fairly straightforward, because the >> tty_operations "ioctl()" function pointer only gets 'struct tty *'. >> >> So in the TIOCGPTPEER path, we don't actually have access to the file >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds wrote: > > Argh. And it's *not* fairly straightforward, because the > tty_operations "ioctl()" function pointer only gets 'struct tty *'. > > So in the TIOCGPTPEER path, we don't actually have access to the file >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:49 PM, Linus Torvalds wrote: > > Argh. And it's *not* fairly straightforward, because the > tty_operations "ioctl()" function pointer only gets 'struct tty *'. > > So in the TIOCGPTPEER path, we don't actually have access to the file > pointer of the fd we're doing the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:32 PM, Linus Torvalds wrote: > > It should all be _fairly_ straightforward, but it's definitely a > rather bigger change than that "just fix the path" patch was. Argh. And it's *not* fairly straightforward, because the tty_operations

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:32 PM, Linus Torvalds wrote: > > It should all be _fairly_ straightforward, but it's definitely a > rather bigger change than that "just fix the path" patch was. Argh. And it's *not* fairly straightforward, because the tty_operations "ioctl()" function pointer only gets

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:25 PM, Eric W. Biederman wrote: > > The new behavior is that when we open ptmx we cache a path the slave > pty. Yes. It's not strictly "new", though - we've done that for a while, and if you used /dev/pts/ptmx you'd even have had the *right* path

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 6:25 PM, Eric W. Biederman wrote: > > The new behavior is that when we open ptmx we cache a path the slave > pty. Yes. It's not strictly "new", though - we've done that for a while, and if you used /dev/pts/ptmx you'd even have had the *right* path for a while ;) And

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 5:24 PM, Stefan Lippers-Hollmann wrote: >> >> This patch[1] as part of 4.13-rc6 (up to, at least, >> v4.13-rc6-45-g6470812e2226) introduces a regression for me when using >> pbuilder 0.228.7[2] (a

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 23, 2017 at 5:24 PM, Stefan Lippers-Hollmann wrote: >> >> This patch[1] as part of 4.13-rc6 (up to, at least, >> v4.13-rc6-45-g6470812e2226) introduces a regression for me when using >> pbuilder 0.228.7[2] (a helper to build Debian packages in a chroot and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 5:42 PM, Linus Torvalds wrote: > > Let me try to think about alteratives. Clearly this is a regression > and I need to fix it, I just need to figure out _how_. Ok, sadly, I think it's unfixable with the current model. We literally used to

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 5:42 PM, Linus Torvalds wrote: > > Let me try to think about alteratives. Clearly this is a regression > and I need to fix it, I just need to figure out _how_. Ok, sadly, I think it's unfixable with the current model. We literally used to keep the wrong 'struct path'

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 5:24 PM, Stefan Lippers-Hollmann wrote: > > This patch[1] as part of 4.13-rc6 (up to, at least, > v4.13-rc6-45-g6470812e2226) introduces a regression for me when using > pbuilder 0.228.7[2] (a helper to build Debian packages in a chroot and > to create and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 5:24 PM, Stefan Lippers-Hollmann wrote: > > This patch[1] as part of 4.13-rc6 (up to, at least, > v4.13-rc6-45-g6470812e2226) introduces a regression for me when using > pbuilder 0.228.7[2] (a helper to build Debian packages in a chroot and > to create and update its

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi On 2017-08-16, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman > wrote: [...] > Maybe this attached patch is better anyway. It's smaller, because it > keeps more closely to the old code, and just adds a mntput() in all > the exit cases, and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Hi On 2017-08-16, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman > wrote: [...] > Maybe this attached patch is better anyway. It's smaller, because it > keeps more closely to the old code, and just adds a mntput() in all > the exit cases, and depends on the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 10:31:53AM -0500, Eric W. Biederman wrote: > Christian Brauner writes: > > > On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds > > wrote: > >> On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner > >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 23, 2017 at 10:31:53AM -0500, Eric W. Biederman wrote: > Christian Brauner writes: > > > On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds > > wrote: > >> On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner > >> wrote: > And Christian, if you can beat on this, that would be

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Christian Brauner writes: > On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds > wrote: >> On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner >> wrote: And Christian, if you can beat on this,

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Christian Brauner writes: > On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds > wrote: >> On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner >> wrote: And Christian, if you can beat on this, that would be good. >>> >>> Yes, I can pound on this nicely with liblxc. We have patch >>> (

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds > wrote: >> >> So the fact that we _don't_ get the right pathname for the pts entry >> here means that something got screwed up in setting filp->f_path to >>

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds > wrote: >> >> So the fact that we _don't_ get the right pathname for the pts entry >> here means that something got screwed up in setting filp->f_path to >> the right thing. We have all the code in place that _tries_ to

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman > wrote: >> >> *Blink* You are right I missed that. >> >> In which case I am concerned about failures that make it to err_release. >> Unless I am missing something

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman > wrote: >> >> *Blink* You are right I missed that. >> >> In which case I am concerned about failures that make it to err_release. >> Unless I am missing something (again) failures that jump to err_release >> won't call

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman wrote: > > *Blink* You are right I missed that. > > In which case I am concerned about failures that make it to err_release. > Unless I am missing something (again) failures that jump to err_release > won't call mntput and

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman wrote: > > *Blink* You are right I missed that. > > In which case I am concerned about failures that make it to err_release. > Unless I am missing something (again) failures that jump to err_release > won't call mntput and will result in a mnt

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 3:46 PM, Eric W. Biederman > wrote: >>> tty->link->driver_data = pts_path; >>> >>> retval = ptm_driver->ops->open(tty, filp); >> ^^^ >> >> If this open fails the code

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 3:46 PM, Eric W. Biederman > wrote: >>> tty->link->driver_data = pts_path; >>> >>> retval = ptm_driver->ops->open(tty, filp); >> ^^^ >> >> If this open fails the code jumps to err_put_path which falls >> through into

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 3:46 PM, Eric W. Biederman wrote: >> tty->link->driver_data = pts_path; >> >> retval = ptm_driver->ops->open(tty, filp); > ^^^ > > If this open fails the code jumps to err_put_path which falls > through into out_put_fsi. No

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 3:46 PM, Eric W. Biederman wrote: >> tty->link->driver_data = pts_path; >> >> retval = ptm_driver->ops->open(tty, filp); > ^^^ > > If this open fails the code jumps to err_put_path which falls > through into out_put_fsi. No it doesn't.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds > wrote: >> >> I suspect the easiest fix is to just add a "mnt" argument to >> devpts_acquire(), It shouldn't be too painful. Let me try. > > Ok, here's a

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

Linus Torvalds writes: > On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds > wrote: >> >> I suspect the easiest fix is to just add a "mnt" argument to >> devpts_acquire(), It shouldn't be too painful. Let me try. > > Ok, here's a *very* lightly tested patch. It might have new bugs, but > it

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner > wrote: >>> And Christian, if you can beat on this, that would be good. >> >> Yes, I can pound on this nicely with liblxc.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:45 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner > wrote: >>> And Christian, if you can beat on this, that would be good. >> >> Yes, I can pound on this nicely with liblxc. We have patch >> ( https://github.com/lxc/lxc/pull/1728 ) up

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:55 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 2:45 PM, Linus Torvalds > wrote: >> >> But it would be good to just test this in general too, and make sure I >> didn't screw up some reference count or

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:55 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 2:45 PM, Linus Torvalds > wrote: >> >> But it would be good to just test this in general too, and make sure I >> didn't screw up some reference count or something. The patch *looks* >> obviously correct, but ... >

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 2:45 PM, Linus Torvalds wrote: > > But it would be good to just test this in general too, and make sure I > didn't screw up some reference count or something. The patch *looks* > obviously correct, but ... Side note: I suspect it should be

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 2:45 PM, Linus Torvalds wrote: > > But it would be good to just test this in general too, and make sure I > didn't screw up some reference count or something. The patch *looks* > obviously correct, but ... Side note: I suspect it should be marked for stable, but it's

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner wrote: >> And Christian, if you can beat on this, that would be good. > > Yes, I can pound on this nicely with liblxc. We have patch > ( https://github.com/lxc/lxc/pull/1728 ) up for review that > allocates pty

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 2:37 PM, Christian Brauner wrote: >> And Christian, if you can beat on this, that would be good. > > Yes, I can pound on this nicely with liblxc. We have patch > ( https://github.com/lxc/lxc/pull/1728 ) up for review that > allocates pty fds from private devpts mounts in

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:03 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds > wrote: >> >> I suspect the easiest fix is to just add a "mnt" argument to >> devpts_acquire(), It shouldn't be too painful.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:03 PM, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds > wrote: >> >> I suspect the easiest fix is to just add a "mnt" argument to >> devpts_acquire(), It shouldn't be too painful. Let me try. > > Ok, here's a *very* lightly tested patch. It

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds wrote: > > I suspect the easiest fix is to just add a "mnt" argument to > devpts_acquire(), It shouldn't be too painful. Let me try. Ok, here's a *very* lightly tested patch. It might have new bugs, but it makes your

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds wrote: > > I suspect the easiest fix is to just add a "mnt" argument to > devpts_acquire(), It shouldn't be too painful. Let me try. Ok, here's a *very* lightly tested patch. It might have new bugs, but it makes your test program DTRT. Al, mind

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 1:19 PM, Linus Torvalds wrote: > > Anyway, I know what's wrong, next step is to figure out what the fix is. Grr, We actually look up the right mnt in devpts_acquire(), but we don't save it. We only save the superblock pointer, because

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 1:19 PM, Linus Torvalds wrote: > > Anyway, I know what's wrong, next step is to figure out what the fix is. Grr, We actually look up the right mnt in devpts_acquire(), but we don't save it. We only save the superblock pointer, because that's traditionally what we used.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds wrote: > > So the fact that we _don't_ get the right pathname for the pts entry > here means that something got screwed up in setting filp->f_path to > the right thing. We have all the code in place that _tries_ to do

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 12:56 PM, Linus Torvalds wrote: > > So the fact that we _don't_ get the right pathname for the pts entry > here means that something got screwed up in setting filp->f_path to > the right thing. We have all the code in place that _tries_ to do it, > but it clearly has a bug

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 12:48 PM, Christian Brauner wrote: > > I thought - and sorry if I'm completely wrong here - that the proc name came > from the open(const char *pathname, ...) call. No. It comes from the path associated with the file descriptor, and is

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 12:48 PM, Christian Brauner wrote: > > I thought - and sorry if I'm completely wrong here - that the proc name came > from the open(const char *pathname, ...) call. No. It comes from the path associated with the file descriptor, and is expanded from the dentry tree.

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:48:48AM -0700, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 11:26 AM, Linus Torvalds > wrote: > > > > Hardcoding "/dev/pts/%d" is something that user space can already do. > > The kernel can and should do better. > > Put another way:

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:48:48AM -0700, Linus Torvalds wrote: > On Wed, Aug 16, 2017 at 11:26 AM, Linus Torvalds > wrote: > > > > Hardcoding "/dev/pts/%d" is something that user space can already do. > > The kernel can and should do better. > > Put another way: there's no point in applying the

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:26 AM, Linus Torvalds wrote: > > Hardcoding "/dev/pts/%d" is something that user space can already do. > The kernel can and should do better. Put another way: there's no point in applying the patch as-is, since existing glibc ptsname()

Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name

On Wed, Aug 16, 2017 at 11:26 AM, Linus Torvalds wrote: > > Hardcoding "/dev/pts/%d" is something that user space can already do. > The kernel can and should do better. Put another way: there's no point in applying the patch as-is, since existing glibc ptsname() does the same thing better and

  1   2   >