| Message ID | 20180426151222.6vw67lwqmu6ffgnw@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sebastian, On 26/04/2018 17:12, Sebastian Andrzej Siewior wrote: > On 2018-04-26 17:06:25 [+0200], To linux-serial@vger.kernel.org wrote: >> It seems not to happen in v4.1.51 but it happens in v4.9 and v4.17-rc2 >> so if it broke accidentally it was not recently. > > This is what I used to check: > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -170,7 +170,7 @@ void free_tty_struct(struct tty_struct *tty) > put_device(tty->dev); > kfree(tty->write_buf); > tty->magic = 0xDEADDEAD; > - kfree(tty); > + strcpy(tty->name, "GONE"); > } > > static inline struct tty_struct *file_tty(struct file *file) > > If this is unknown and a bisect is requested, please let me know. Indeed, this will be appreciated. I'm quite curious to find the commit that led to this. Thanks ! > > Sebastian > Richard
On 2018-04-27 12:31:52 [+0200], Richard Genoud wrote: > Hi Sebastian, Hi, > > If this is unknown and a bisect is requested, please let me know. > Indeed, this will be appreciated. > I'm quite curious to find the commit that led to this. commit 761ed4a94582ab291aa24dcbea4e01e8936488c8 Author: Rob Herring <robh@kernel.org> Date: Mon Aug 22 17:39:10 2016 -0500 tty: serial_core: convert uart_close to use tty_port_close tty_port_close handles much of the common parts of tty close. Convert uart_close to use it and move the serial_core specific parts into tty_port.shutdown function. This will be needed to use tty_port functions directly from in kernel clients. This change causes ops->stop_rx() to be called after uart_wait_until_sent() is called which I think should be fine. Otherwise, the sequence of the close should be the same. Cc: Peter Hurley <peter@hurleysoftware.com> Signed-off-by: Rob Herring <robh@kernel.org> Acked-by: Alan Cox <alan@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> The thread starts at https://www.spinics.net/lists/linux-serial/msg30070.html http://lkml.kernel.org/r/20180426150625.q5tqcb7fzchvkb5d@linutronix.de > Richard Sebastian
On 02/05/2018 21:16, Sebastian Andrzej Siewior wrote: > On 2018-04-27 12:31:52 [+0200], Richard Genoud wrote: >> Hi Sebastian, > Hi, > >>> If this is unknown and a bisect is requested, please let me know. >> Indeed, this will be appreciated. >> I'm quite curious to find the commit that led to this. > > commit 761ed4a94582ab291aa24dcbea4e01e8936488c8 > Author: Rob Herring <robh@kernel.org> > Date: Mon Aug 22 17:39:10 2016 -0500 > > tty: serial_core: convert uart_close to use tty_port_close > > tty_port_close handles much of the common parts of tty close. Convert > uart_close to use it and move the serial_core specific parts into > tty_port.shutdown function. This will be needed to use tty_port functions > directly from in kernel clients. > > This change causes ops->stop_rx() to be called after uart_wait_until_sent() > is called which I think should be fine. Otherwise, the sequence of the > close should be the same. > > Cc: Peter Hurley <peter@hurleysoftware.com> > Signed-off-by: Rob Herring <robh@kernel.org> > Acked-by: Alan Cox <alan@linux.intel.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > The thread starts at > https://www.spinics.net/lists/linux-serial/msg30070.html > http://lkml.kernel.org/r/20180426150625.q5tqcb7fzchvkb5d@linutronix.de Thanks for this hunting session ! Could resend your patch with Fixes: in the commit message ? BTW, I didn't manage to reproduce the behavior you describe, could you give me your .config and describe a little more how you manage to trigger this bug ? (do you use the console on serial debug ? which board ? ) Thanks !
On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote: > Could resend your patch with Fixes: in the commit message ? will do shortly. > BTW, I didn't manage to reproduce the behavior you describe, could you > give me your .config and describe a little more how you manage to > trigger this bug ? .config sent offlist. Did you not reproduce this even with the second/debug patch I've sent? > (do you use the console on serial debug ? which board ? ) up to date debian sid booting with systemd. After boot completed I do "cat /proc/interrupts" and check for the "gone" string. This is [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait [ 0.520000] AT91: Detected SoC family: sama5d3 [ 0.520000] AT91: Detected SoC: sama5d36, revision 2 at91-sama5d3_xplained.dtb Is this enough? > Thanks ! Sebastian
On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote: > On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote: >> Could resend your patch with Fixes: in the commit message ? > > will do shortly. > >> BTW, I didn't manage to reproduce the behavior you describe, could you >> give me your .config and describe a little more how you manage to >> trigger this bug ? > .config sent offlist. > Did you not reproduce this even with the second/debug patch I've sent? Nope. (I tried on a sam9g35 with buildroot/busybox) I have a sama5d3 with a debian as a home server, I'll give it a try. > >> (do you use the console on serial debug ? which board ? ) > up to date debian sid booting with systemd. After boot completed I do > "cat /proc/interrupts" and check for the "gone" string. > This is > [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained > [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait > [ 0.520000] AT91: Detected SoC family: sama5d3 > [ 0.520000] AT91: Detected SoC: sama5d36, revision 2 > > at91-sama5d3_xplained.dtb > > Is this enough? Great, Thanks !
On 03/05/2018 15:34, Richard Genoud wrote: > On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote: >> On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote: >>> Could resend your patch with Fixes: in the commit message ? >> >> will do shortly. >> >>> BTW, I didn't manage to reproduce the behavior you describe, could you >>> give me your .config and describe a little more how you manage to >>> trigger this bug ? >> .config sent offlist. >> Did you not reproduce this even with the second/debug patch I've sent? > Nope. (I tried on a sam9g35 with buildroot/busybox) > > I have a sama5d3 with a debian as a home server, I'll give it a try. Ok, I reproduced the use-after-free on a SAMA5D2 and on a SAMA5D3, both with debian (sid / stretch). After the patch, no mode funny things in /proc/interrupts. > >> >>> (do you use the console on serial debug ? which board ? ) >> up to date debian sid booting with systemd. After boot completed I do >> "cat /proc/interrupts" and check for the "gone" string. >> This is >> [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained >> [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait >> [ 0.520000] AT91: Detected SoC family: sama5d3 >> [ 0.520000] AT91: Detected SoC: sama5d36, revision 2 >> >> at91-sama5d3_xplained.dtb >> >> Is this enough? > > Great, > > Thanks ! >
On 03/05/2018 15:34, Richard Genoud wrote: > On 03/05/2018 14:44, Sebastian Andrzej Siewior wrote: >> On 2018-05-03 14:36:03 [+0200], Richard Genoud wrote: >>> Could resend your patch with Fixes: in the commit message ? >> >> will do shortly. Actually, this fix can only be applied on stable kernels 4.14+, because uart_port->name was introduced in 4.12 by commit f7048b15900f ("tty: serial_core: Add name field to uart_port struct") >> >>> BTW, I didn't manage to reproduce the behavior you describe, could you >>> give me your .config and describe a little more how you manage to >>> trigger this bug ? >> .config sent offlist. >> Did you not reproduce this even with the second/debug patch I've sent? > Nope. (I tried on a sam9g35 with buildroot/busybox) > > I have a sama5d3 with a debian as a home server, I'll give it a try. > >> >>> (do you use the console on serial debug ? which board ? ) >> up to date debian sid booting with systemd. After boot completed I do >> "cat /proc/interrupts" and check for the "gone" string. >> This is >> [ 0.000000] OF: fdt:Machine model: SAMA5D3 Xplained >> [ 0.000000] Kernel command line: earlyprintk=ttyS0,115200 console=ttyS0,115200 root=/dev/mmcblk0p2 rootwait >> [ 0.520000] AT91: Detected SoC family: sama5d3 >> [ 0.520000] AT91: Detected SoC: sama5d36, revision 2 >> >> at91-sama5d3_xplained.dtb >> >> Is this enough? > > Great, > > Thanks ! >
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -170,7 +170,7 @@ void free_tty_struct(struct tty_struct *tty) put_device(tty->dev); kfree(tty->write_buf); tty->magic = 0xDEADDEAD; - kfree(tty); + strcpy(tty->name, "GONE"); } static inline struct tty_struct *file_tty(struct file *file)