Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1291812 > unrolled thread
| Started by | "Herton R. Krzesinski" <herton@redhat.com> |
|---|---|
| First post | 2015-12-15 04:30 +0100 |
| Last post | 2015-12-15 18:50 +0100 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
pty: fix use after free/oops at pty_unix98_shutdown "Herton R. Krzesinski" <herton@redhat.com> - 2015-12-15 04:30 +0100
Re: pty: fix use after free/oops at pty_unix98_shutdown Peter Hurley <peter@hurleysoftware.com> - 2015-12-15 17:20 +0100
Re: pty: fix use after free/oops at pty_unix98_shutdown "Herton R. Krzesinski" <herton@redhat.com> - 2015-12-15 17:40 +0100
Re: pty: fix use after free/oops at pty_unix98_shutdown Peter Hurley <peter@hurleysoftware.com> - 2015-12-15 18:30 +0100
Re: pty: fix use after free/oops at pty_unix98_shutdown "Herton R. Krzesinski" <herton@redhat.com> - 2015-12-15 18:50 +0100
| From | "Herton R. Krzesinski" <herton@redhat.com> |
|---|---|
| Date | 2015-12-15 04:30 +0100 |
| Subject | pty: fix use after free/oops at pty_unix98_shutdown |
| Message-ID | <qFOtj-6Va-3@gated-at.bofh.it> |
Hi,
recently I got a report of a crash at pty_unix98_shutdown. after analyzing the
issue, I managed to create a small reproducer:
$ cat test.sh
#!/bin/sh
while true; do
find /sys
./dopty
echo 2 > /proc/sys/vm/drop_caches
ps aux
sleep 40
done
$ cat dopty.c
#define _XOPEN_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <termios.h>
#include <errno.h>
int main(int argc, char **argv)
{
pid_t pid;
int ptm_fd, pty_fd, tty_fd, tout;
char *pty_name;
int c = 0;
while (c < 100) {
c++;
pid = fork();
if (pid != 0)
continue;
daemon(1, 0);
ptm_fd = posix_openpt(O_RDWR);
if (ptm_fd < 0)
return -1;
grantpt(ptm_fd);
unlockpt(ptm_fd);
pty_name = ptsname(ptm_fd);
pty_fd = open(pty_name, O_RDWR);
tty_fd = open("/dev/tty", O_RDWR);
pid = fork();
if (pid == 0) {
ioctl(tty_fd, TIOCNOTTY, NULL);
setsid();
close(pty_fd);
close(ptm_fd);
sleep(60);
return 0;
}
sleep(10);
return 0;
}
sleep(30);
return 0;
}
Running test.sh above, after second iteration I'm able to get the following
oops at devpts_kill_index:
[ 1741.842926] test.sh (8786): drop_caches: 2
[ 1812.417114] test.sh (8786): drop_caches: 2
[ 1842.405991] BUG: unable to handle kernel NULL pointer dereference at 0000000000000060
[ 1842.406055] IP: [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406110] PGD 0
[ 1842.406123] Oops: 0000 [#1] SMP
[ 1842.406148] Modules linked in: 8021q mrp garp stp llc nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6t
able_filter ip6_tables binfmt_misc ppdev parport_pc parport floppy joydev tpm_tis tpm virtio_balloon serio_raw virtio_console virtio_net iosf_mbi crct10dif_pclmul crc32_pclmul
pcspkr qxl ttm drm_kms_helper drm snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore i2c_piix4 virt
io_blk crc32c_intel virtio_pci virtio_ring virtio pata_acpi ata_generic [last unloaded: speedstep_lib]
[ 1842.406383] CPU: 2 PID: 9204 Comm: dopty Not tainted 4.4.0-rc5 #1
[ 1842.406404] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014
[ 1842.406427] task: ffff880039da3a00 ti: ffff880037a7c000 task.ti: ffff880037a7c000
[ 1842.406445] RIP: 0010:[<ffffffff8129f36d>] [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406473] RSP: 0018:ffff880037a7fc98 EFLAGS: 00010282
[ 1842.406487] RAX: 0000000000000000 RBX: ffff880039e75c00 RCX: 00000001810000fd
[ 1842.406504] RDX: 00000000ffffffff RSI: 0000000000000004 RDI: ffff88003d0cc6a8
[ 1842.406521] RBP: ffff880037a7fca8 R08: ffffea0000ebcc50 R09: 0000000000000001
[ 1842.406539] R10: ffffea0000ebcc58 R11: 0000000000000000 R12: 0000000000000004
[ 1842.406556] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1842.406573] FS: 0000000000000000(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
[ 1842.406611] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1842.406634] CR2: 0000000000000060 CR3: 0000000001c0a000 CR4: 00000000001406e0
[ 1842.406665] Stack:
[ 1842.406677] ffff880039e75c00 0000000000000000 ffff880037a7fcb8 ffffffff814802f8
[ 1842.406715] ffff880037a7fce8 ffffffff814749fe ffff880000000000 ffffffff8177ce56
[ 1842.406785] 0000000000000000 ffff880039e75c00 ffff880037a7fd88 ffffffff81475ada
[ 1842.406821] Call Trace:
[ 1842.406821] [<ffffffff814802f8>] pty_unix98_shutdown+0x18/0x20
[ 1842.406821] [<ffffffff814749fe>] release_tty+0x3e/0xe0
[ 1842.406821] [<ffffffff8177ce56>] ? mutex_lock+0x16/0x40
[ 1842.406821] [<ffffffff81475ada>] tty_release+0x44a/0x580
[ 1842.406821] [<ffffffff8121e3e5>] __fput+0xb5/0x200
[ 1842.406821] [<ffffffff8121e5de>] ____fput+0xe/0x10
[ 1842.406821] [<ffffffff810b2eb8>] task_work_run+0x68/0xa0
[ 1842.406821] [<ffffffff8109a3e0>] do_exit+0x320/0x670
[ 1842.406821] [<ffffffff810673d4>] ? __do_page_fault+0x1a4/0x450
[ 1842.406821] [<ffffffff811361d0>] ? __audit_syscall_entry+0xb0/0x110
[ 1842.406821] [<ffffffff81003376>] ? do_audit_syscall_entry+0x66/0x70
[ 1842.406821] [<ffffffff8109a781>] do_group_exit+0x51/0xc0
[ 1842.406821] [<ffffffff8109a807>] SyS_exit_group+0x17/0x20
[ 1842.406821] [<ffffffff8177f2ee>] entry_SYSCALL_64_fastpath+0x12/0x71
[ 1842.406821] Code: 48 c7 c3 f4 ff ff ff eb 84 e8 10 7b df ff 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 66 66 66 66 90 48 8b 47 28 41 89 f4 <48> 81 78 60 d1 1c 00 00 74 12 48 8b 15 4a bd d6 00 31 c0 48 85
[ 1842.406821] RIP [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406821] RSP <ffff880037a7fc98>
[ 1842.406821] CR2: 0000000000000060
The problem above is that on final close, pty_unix98_shutdown may give an stale/
already evicted inode from tty->driver_data. Easily to happen as the test case
shows, in case all opened /dev/pts/N references are closed before "/dev/tty". I
also expect in a rare case where all ptmx references are gone/closed, this also
could happen on final close when the master tty is given to pty_unix98_shutdown.
Following this is a proposed fix to the problem, I tested here and it fixed the
issue.
Please take a look and review, thanks!
Regards,
Herton.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2015-12-15 17:20 +0100 |
| Message-ID | <qG0ut-6y5-9@gated-at.bofh.it> |
| In reply to | #1291812 |
Hi Herton,
On 12/14/2015 07:29 PM, Herton R. Krzesinski wrote:
> Hi,
>
> recently I got a report of a crash at pty_unix98_shutdown. after analyzing the
> issue, I managed to create a small reproducer:
>
> $ cat test.sh
> #!/bin/sh
>
> while true; do
> find /sys
> ./dopty
> echo 2 > /proc/sys/vm/drop_caches
> ps aux
> sleep 40
> done
>
> $ cat dopty.c
> #define _XOPEN_SOURCE
> #include <unistd.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <termios.h>
> #include <errno.h>
>
> int main(int argc, char **argv)
> {
> pid_t pid;
> int ptm_fd, pty_fd, tty_fd, tout;
> char *pty_name;
> int c = 0;
>
> while (c < 100) {
> c++;
> pid = fork();
> if (pid != 0)
> continue;
> daemon(1, 0);
> ptm_fd = posix_openpt(O_RDWR);
> if (ptm_fd < 0)
> return -1;
> grantpt(ptm_fd);
> unlockpt(ptm_fd);
> pty_name = ptsname(ptm_fd);
> pty_fd = open(pty_name, O_RDWR);
> tty_fd = open("/dev/tty", O_RDWR);
> pid = fork();
> if (pid == 0) {
> ioctl(tty_fd, TIOCNOTTY, NULL);
> setsid();
> close(pty_fd);
> close(ptm_fd);
> sleep(60);
> return 0;
> }
> sleep(10);
> return 0;
> }
> sleep(30);
> return 0;
> }
Thanks for the reproducer!
> Running test.sh above, after second iteration I'm able to get the following
> oops at devpts_kill_index:
>
> [ 1741.842926] test.sh (8786): drop_caches: 2
> [ 1812.417114] test.sh (8786): drop_caches: 2
> [ 1842.405991] BUG: unable to handle kernel NULL pointer dereference at 0000000000000060
> [ 1842.406055] IP: [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
> [ 1842.406110] PGD 0
> [ 1842.406123] Oops: 0000 [#1] SMP
> [ 1842.406148] Modules linked in: 8021q mrp garp stp llc nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6t
> able_filter ip6_tables binfmt_misc ppdev parport_pc parport floppy joydev tpm_tis tpm virtio_balloon serio_raw virtio_console virtio_net iosf_mbi crct10dif_pclmul crc32_pclmul
> pcspkr qxl ttm drm_kms_helper drm snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore i2c_piix4 virt
> io_blk crc32c_intel virtio_pci virtio_ring virtio pata_acpi ata_generic [last unloaded: speedstep_lib]
> [ 1842.406383] CPU: 2 PID: 9204 Comm: dopty Not tainted 4.4.0-rc5 #1
> [ 1842.406404] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014
> [ 1842.406427] task: ffff880039da3a00 ti: ffff880037a7c000 task.ti: ffff880037a7c000
> [ 1842.406445] RIP: 0010:[<ffffffff8129f36d>] [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
> [ 1842.406473] RSP: 0018:ffff880037a7fc98 EFLAGS: 00010282
> [ 1842.406487] RAX: 0000000000000000 RBX: ffff880039e75c00 RCX: 00000001810000fd
> [ 1842.406504] RDX: 00000000ffffffff RSI: 0000000000000004 RDI: ffff88003d0cc6a8
> [ 1842.406521] RBP: ffff880037a7fca8 R08: ffffea0000ebcc50 R09: 0000000000000001
> [ 1842.406539] R10: ffffea0000ebcc58 R11: 0000000000000000 R12: 0000000000000004
> [ 1842.406556] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 1842.406573] FS: 0000000000000000(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
> [ 1842.406611] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1842.406634] CR2: 0000000000000060 CR3: 0000000001c0a000 CR4: 00000000001406e0
> [ 1842.406665] Stack:
> [ 1842.406677] ffff880039e75c00 0000000000000000 ffff880037a7fcb8 ffffffff814802f8
> [ 1842.406715] ffff880037a7fce8 ffffffff814749fe ffff880000000000 ffffffff8177ce56
> [ 1842.406785] 0000000000000000 ffff880039e75c00 ffff880037a7fd88 ffffffff81475ada
> [ 1842.406821] Call Trace:
> [ 1842.406821] [<ffffffff814802f8>] pty_unix98_shutdown+0x18/0x20
> [ 1842.406821] [<ffffffff814749fe>] release_tty+0x3e/0xe0
> [ 1842.406821] [<ffffffff8177ce56>] ? mutex_lock+0x16/0x40
> [ 1842.406821] [<ffffffff81475ada>] tty_release+0x44a/0x580
> [ 1842.406821] [<ffffffff8121e3e5>] __fput+0xb5/0x200
> [ 1842.406821] [<ffffffff8121e5de>] ____fput+0xe/0x10
> [ 1842.406821] [<ffffffff810b2eb8>] task_work_run+0x68/0xa0
> [ 1842.406821] [<ffffffff8109a3e0>] do_exit+0x320/0x670
> [ 1842.406821] [<ffffffff810673d4>] ? __do_page_fault+0x1a4/0x450
> [ 1842.406821] [<ffffffff811361d0>] ? __audit_syscall_entry+0xb0/0x110
> [ 1842.406821] [<ffffffff81003376>] ? do_audit_syscall_entry+0x66/0x70
> [ 1842.406821] [<ffffffff8109a781>] do_group_exit+0x51/0xc0
> [ 1842.406821] [<ffffffff8109a807>] SyS_exit_group+0x17/0x20
> [ 1842.406821] [<ffffffff8177f2ee>] entry_SYSCALL_64_fastpath+0x12/0x71
> [ 1842.406821] Code: 48 c7 c3 f4 ff ff ff eb 84 e8 10 7b df ff 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 66 66 66 66 90 48 8b 47 28 41 89 f4 <48> 81 78 60 d1 1c 00 00 74 12 48 8b 15 4a bd d6 00 31 c0 48 85
> [ 1842.406821] RIP [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
> [ 1842.406821] RSP <ffff880037a7fc98>
> [ 1842.406821] CR2: 0000000000000060
>
> The problem above is that on final close, pty_unix98_shutdown may give an stale/
> already evicted inode from tty->driver_data. Easily to happen as the test case
> shows, in case all opened /dev/pts/N references are closed before "/dev/tty".
Yeah, I agree with your analysis here. Nothing bumps the slave inode reference for
which the current tty is a proxy.
So if the current tty is the last released, that means all the master and slave
inodes have been released. So the inode still in tty->driver_data for the slave is
stale.
> I also expect in a rare case where all ptmx references are gone/closed, this also
> could happen on final close when the master tty is given to pty_unix98_shutdown.
This logic I'm not following. If the pty master is being released, then the inode
is valid for the release() operation in-progress.
Regards,
Peter Hurley
> Following this is a proposed fix to the problem, I tested here and it fixed the
> issue.
>
> Please take a look and review, thanks!
Looking at it now.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Herton R. Krzesinski" <herton@redhat.com> |
|---|---|
| Date | 2015-12-15 17:40 +0100 |
| Message-ID | <qG0NR-6Hh-53@gated-at.bofh.it> |
| In reply to | #1292323 |
On Tue, Dec 15, 2015 at 08:17:56AM -0800, Peter Hurley wrote:
> > I also expect in a rare case where all ptmx references are gone/closed, this also
> > could happen on final close when the master tty is given to pty_unix98_shutdown.
>
> This logic I'm not following. If the pty master is being released, then the inode
> is valid for the release() operation in-progress.
Hi Peter,
yes, you're right if you are eg. closing the /dev/ptmx or /dev/pts/ptmx file
previously opened. But I thought and refer above to the case where for example
you are closing /dev/tty and that's the final close and there is no other
process in the system with the /dev/{,*/}ptmx opened, the inode which referenced
the previously opened ptmx could be gone. It would be rare though since in a
running system any logged in user eg. through ssh or with a terminal open in X
will have at least a ptmx device opened.
>
> Regards,
> Peter Hurley
>
--
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2015-12-15 18:30 +0100 |
| Message-ID | <qG1Ae-7eq-11@gated-at.bofh.it> |
| In reply to | #1292336 |
On 12/15/2015 08:36 AM, Herton R. Krzesinski wrote:
> On Tue, Dec 15, 2015 at 08:17:56AM -0800, Peter Hurley wrote:
>>> I also expect in a rare case where all ptmx references are gone/closed, this also
>>> could happen on final close when the master tty is given to pty_unix98_shutdown.
>>
>> This logic I'm not following. If the pty master is being released, then the inode
>> is valid for the release() operation in-progress.
>
> Hi Peter,
>
> yes, you're right if you are eg. closing the /dev/ptmx or /dev/pts/ptmx file
> previously opened. But I thought and refer above to the case where for example
> you are closing /dev/tty and that's the final close and there is no other
> process in the system with the /dev/{,*/}ptmx opened, the inode which referenced
> the previously opened ptmx could be gone. It would be rare though since in a
> running system any logged in user eg. through ssh or with a terminal open in X
> will have at least a ptmx device opened.
/dev/tty can never be an alias for /dev/ptmx in Linux: a master pty cannot be
a controlling terminal. So if the master pty is being released it will always be
with the /dev/ptmx inode.
Regards,
Peter Hurley
PS - for the purpose of this discussion, /dev/pts/ptmx is equivalent.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Herton R. Krzesinski" <herton@redhat.com> |
|---|---|
| Date | 2015-12-15 18:50 +0100 |
| Message-ID | <qG1Tz-7mv-13@gated-at.bofh.it> |
| In reply to | #1292379 |
On Tue, Dec 15, 2015 at 09:28:28AM -0800, Peter Hurley wrote:
> On 12/15/2015 08:36 AM, Herton R. Krzesinski wrote:
> > On Tue, Dec 15, 2015 at 08:17:56AM -0800, Peter Hurley wrote:
> >>> I also expect in a rare case where all ptmx references are gone/closed, this also
> >>> could happen on final close when the master tty is given to pty_unix98_shutdown.
> >>
> >> This logic I'm not following. If the pty master is being released, then the inode
> >> is valid for the release() operation in-progress.
> >
> > Hi Peter,
> >
> > yes, you're right if you are eg. closing the /dev/ptmx or /dev/pts/ptmx file
> > previously opened. But I thought and refer above to the case where for example
> > you are closing /dev/tty and that's the final close and there is no other
> > process in the system with the /dev/{,*/}ptmx opened, the inode which referenced
> > the previously opened ptmx could be gone. It would be rare though since in a
> > running system any logged in user eg. through ssh or with a terminal open in X
> > will have at least a ptmx device opened.
>
> /dev/tty can never be an alias for /dev/ptmx in Linux: a master pty cannot be
> a controlling terminal. So if the master pty is being released it will always be
> with the /dev/ptmx inode.
Indeed, pty_unix98_shutdown in case of /dev/tty close will be called with slave
pty tty_struct. Sorry about my previous confusing/wrong statement. My concern is
only valid in case final /dev/tty close used the ptmx inode instead of slave_inode
created at ptmx_open, which was not the case before/with current code, but is
the case when applying my patch, thus I grab the reference to the inode.
>
> Regards,
> Peter Hurley
>
> PS - for the purpose of this discussion, /dev/pts/ptmx is equivalent.
--
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web