n_tty_flush_buffer can happen in parallel with n_tty_close that the
tty->disc_data will be set to NULL. n_tty_flush_buffer accesses
tty->disc_data, so we must prevent n_tty_close clear tty->disc_data
while n_tty_flush_buffer has a non-NULL view of tty->disc_data.
So we need to make sure that accesses to disc_data are atomic using
tty->termios_rwsem.
There is an example I meet:
When n_tty_flush_buffer accesses tty struct, the disc_data is right.
However, then reset_buffer_flags accesses tty->disc_data, disc_data
become NULL, So kernel crash when accesses tty->disc_data->real_tail.
I guess there could be another thread change tty->disc_data to NULL,
and during N_TTY line discipline, n_tty_close will set tty->disc_data
to be NULL. So use tty->termios_rwsem to protect disc_data between close
and flush_buffer.
IP: reset_buffer_flags+0x9/0xf0
PGD 0 P4D 0
Oops: 0002 [#1] SMP
CPU: 23 PID: 2087626 Comm: (agetty) Kdump: loaded Tainted: G
Hardware name: UNISINSIGHT X3036P-G3/ST01M2C7S, BIOS 2.00.13 01/11/2019
task: ffff9c4e9da71e80 task.stack: ffffb30cfe898000
RIP: 0010:reset_buffer_flags+0x9/0xf0
RSP: 0018:ffffb30cfe89bca8 EFLAGS: 00010246
RAX: ffff9c4e9da71e80 RBX: ffff9c368d1bac00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff9c4ea17b50f0 RDI: 0000000000000000
RBP: ffffb30cfe89bcc8 R08: 0000000000000100 R09: 0000000000000001
R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c368d1bacc0
R13: ffff9c20cfd18428 R14: ffff9c4ea17b50f0 R15: ffff9c368d1bac00
FS: 00007f9fbbe97940(0000) GS:ffff9c375c740000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000002260 CR3: 0000002f72233003 CR4: 00000000007606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
? n_tty_flush_buffer+0x2a/0x60
tty_buffer_flush+0x76/0x90
tty_ldisc_flush+0x22/0x40
vt_ioctl+0x5a7/0x10b0
? n_tty_ioctl_helper+0x27/0x110
tty_ioctl+0xef/0x8c0
do_vfs_ioctl+0xa7/0x5e0
? __audit_syscall_entry+0xaf/0x100
? syscall_trace_enter+0x1d0/0x2b0
SyS_ioctl+0x79/0x90
do_syscall_64+0x6c/0x1b0
entry_SYSCALL64_slow_path+0x25/0x25
n_tty_flush_buffer --->tty->disc_data is OK
->reset_buffer_flags -->tty->disc_data is NULL
Signed-off-by: Yan.Gao <gao.yanB@h3c.com>
Reviewed-by: Xianting Tian <tian.xianting@h3c.com>
Link: https://lore.kernel.org/r/20201210022507.30729-1-gao.yanB@h3c.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove the tty_vhangup() from the pty code and just release the
redirect. The tty_vhangup() results in data loss and data out of order
issues.
If you write to a pty master an immediately close the pty master, the
receiver might get a chunk of data dropped, but then receive some later
data. That's obviously something rather unexpected for a user. It
certainly confused my test program.
It turns out that tty_vhangup() on the slave pty gets called from
pty_close(), and that causes the data on the slave side to be flushed,
but due to races more data can be copied into the slave side's buffer
after that. Consider the following sequence:
thread1 thread2 thread3
------- ------- -------
| |-write data into buffer,
| | n_tty buffer is filled
| | along with other buffers
| |-pty_close(master)
| |--tty_vhangup(slave)
| |---tty_ldisc_hangup()
| |----n_tty_flush_buffer()
| |-----reset_buffer_flags()
|-n_tty_read() |
|--up_read(&tty->termios_rwsem);
| |------down_read(&tty->termios_rwsem)
| |------clear n_tty buffer contents
| |------up_read(&tty->termios_rwsem)
|--tty_buffer_flush_work() |
|--schedules work calling |
| flush_to_ldisc() |
| |-flush_to_ldisc()
| |--receive_buf()
| |---tty_port_default_receive_buf()
| |----tty_ldisc_receive_buf()
| |-----n_tty_receive_buf2()
| |------n_tty_receive_buf_common()
| |-------down_read(&tty->termios_rwsem)
| |-------__receive_buf()
| | copies data into n_tty buffer
| |-------up_read(&tty->termios_rwsem)
|--down_read(&tty->termios_rwsem)
|--copy buffer data to user
>From this sequence, you can see that thread2 writes to the buffer then
only clears the part of the buffer in n_tty. The n_tty receive buffer
code then copies more data into the n_tty buffer.
But part of the vhangup, releasing the redirect, is still required to
avoid issues with consoles running on pty slaves. So do that.
As far as I can tell, that is all that should be required.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Link: https://lore.kernel.org/r/20201124004902.1398477-3-minyard@acm.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The transmission complete error message provides the status of the
ISR_USART_TC bit. This bit, when set, indicates that the transmission
has not been completed.
The bit status indication is not a very understandable information.
The error message sent on console should indicate that the transmission is
not complete, instead of providing USART_TC bit status.
Update the error message and add a comment for better understanding.
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>
Link: https://lore.kernel.org/r/20210106162203.28854-9-erwan.leray@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The comment for conflicting RTS/CTS config refers to "st, hw-flow-ctrl",
but this property is deprecated since the generic RTS/CTS property has
been introduced by the patch 'serial: stm32: Use generic DT binding for
announcing RTS/CTS lines'.
Update the comment to refer to both generic and deprecated RTS/CTS
properties.
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>
Link: https://lore.kernel.org/r/20210106162203.28854-7-erwan.leray@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Update rts-gpios and cts-gpios:
- remove max-items as already defined in serial.yaml
- add a note describing rts-gpios and cts-gpios usage with stm32
Document the use of cts-gpios and rts-gpios for flow control in STM32 UART
controller. These properties can be used instead of 'uart-has-rtscts' or
'st,hw-flow-ctrl' (deprecated) for making use of any gpio pins for flow
control instead of dedicated pins.
It should be noted that both cts-gpios/rts-gpios and 'uart-has-rtscts' or
'st,hw-flow-ctrl' (deprecated) properties cannot co-exist in a design.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>
Link: https://lore.kernel.org/r/20210106162203.28854-6-erwan.leray@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fixes checkpatch --strict warnings and checks:
- checkpatch --strict "Unnecessary parentheses"
- checkpatch --strict "Blank lines aren't necessary before a close brace
- checkpatch --strict "Alignment should match open parenthesis"
- checkpatch --strict "Please don't use multiple blank lines"
- checkpatch --strict "Comparison to NULL could be written ..."
- visual check code ordering warning
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>
Link: https://lore.kernel.org/r/20210106162203.28854-3-erwan.leray@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
DMA initialization error handling is not properly implemented in the
driver.
Fix DMA initialization error handling by:
- moving TX DMA descriptor request error handling in a new dedicated
fallback_err label
- adding error handling to TX DMA descriptor submission
- adding error handling to RX DMA descriptor submission
This patch depends on '24832ca3ee85 ("tty: serial: stm32-usart: Remove set
but unused 'cookie' variables")' which unfortunately doesn't include a
"Fixes" tag.
Fixes: 3489187204 ("serial: stm32: adding dma support")
Signed-off-by: Erwan Le Ray <erwan.leray@foss.st.com>
Link: https://lore.kernel.org/r/20210106162203.28854-2-erwan.leray@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The main purpose of tty_port::low_latency was removed in commit
a9c3f68f3c (tty: Fix low_latency BUG) back in 2014. It was left in
place for drivers as an optional tune knob. But only one driver has been
using it until the previous commit. So remove this misconcept
completely, given there are no users.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-11-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Drop support for these ioctls:
* PIO_FONT, PIO_FONTX
* GIO_FONT, GIO_FONTX
* PIO_FONTRESET
As was demonstrated by commit 90bfdeef83 (tty: make FONTX ioctl use
the tty pointer they were actually passed), these ioctls are not used
from userspace, as:
1) they used to be broken (set up font on current console, not the open
one) and racy (before the commit above)
2) KDFONTOP ioctl is used for years instead
Note that PIO_FONTRESET is defunct on most systems as VGA_CONSOLE is set
on them for ages. That turns on BROKEN_GRAPHICS_PROGRAMS which makes
PIO_FONTRESET just return an error.
We are removing KD_FONT_FLAG_OLD here as it was used only by these
removed ioctls. kd.h header exists both in kernel and uapi headers, so
we can remove the kernel one completely. Everyone includeing kd.h will
now automatically get the uapi one.
There are now unused definitions of the ioctl numbers and "struct
consolefontdesc" in kd.h, but as it is a uapi header, I am not touching
these.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-8-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The constant 20 makes the font sum computation signed which can lead to
sign extensions and signed wraps. It's not much of a problem as we build
with -fno-strict-overflow. But if we ever decide not to, be ready, so
switch the constant to unsigned.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-7-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 5ce2087ed0 (Fix default compose table initialization) fixed
unicode table so that the values are not sign extended. The upstream
(kbd package) chose a different approach. They use hexadecimal values.
So use the same, so that the output of loadkeys and our shipped file
correspond more to each other.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-4-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Introduce a new function tty_kopen_shared() that yields a struct
tty_struct. The semantic difference to tty_kopen() is that the tty is
expected to be used already. So rename tty_kopen() to
tty_kopen_exclusive() for clearness, adapt the single user and put the
common code in a new static helper function.
tty_kopen_shared is to be used to implement an LED trigger for tty
devices in one of the next patches.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20201218104246.591315-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since commit 36e2c7421f ("fs: don't allow splice read/write without
explicit ops") we've required that file operation structures explicitly
enable splice support, rather than falling back to the default handlers.
Most /proc files use the indirect 'struct proc_ops' to describe their
file operations, and were fixed up to support splice earlier in commits
40be821d627c..b24c30c67863, but the mountinfo files interact with the
VFS directly using their own 'struct file_operations' and got missed as
a result.
This adds the necessary support for splice to work for /proc/*/mountinfo
and friends.
Reported-by: Joan Bruguera Micó <joanbrugueram@gmail.com>
Reported-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=209971
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Pull NTB fixes from Jon Mason:
"Bug fix for IDT NTB and Intel NTB LTR management support"
* tag 'ntb-5.11' of git://github.com/jonmason/ntb:
ntb: intel: add Intel NTB LTR vendor support for gen4 NTB
ntb: idt: fix error check in ntb_hw_idt.c
Pull crypto fixes from Herbert Xu:
"Fix a number of autobuild failures due to missing Kconfig
dependencies"
* 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6:
crypto: qat - add CRYPTO_AES to Kconfig dependencies
crypto: keembay - Add dependency on HAS_IOMEM
crypto: keembay - CRYPTO_DEV_KEEMBAY_OCS_AES_SM4 should depend on ARCH_KEEMBAY
Pull objtool fix from Ingo Molnar:
"Fix a segfault that occurs when built with Clang"
* tag 'objtool-urgent-2020-12-27' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
objtool: Fix seg fault with Clang non-section symbols
Pull timer fixes from Ingo Molnar:
"Update/fix two CPU sanity checks in the hotplug and the boot code, and
fix a typo in the Kconfig help text.
[ Context: the first two commits are the result of an ongoing
annotation+review work of (intentional) tick_do_timer_cpu() data
races reported by KCSAN, but the annotations aren't fully cooked
yet ]"
* tag 'timers-urgent-2020-12-27' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
timekeeping: Fix spelling mistake in Kconfig "fullfill" -> "fulfill"
tick/sched: Remove bogus boot "safety" check
tick: Remove pointless cpu valid check in hotplug code
Pull scheduler fix from Ingo Molnar:
"Fix a context switch performance regression"
* tag 'sched-urgent-2020-12-27' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
sched: Optimize finish_lock_switch()
Commit c9a3c4e637 ("mfd: ab8500-debugfs: Remove extraneous curly
brace") removed a left-over curly brace that caused build failures, but
Joe Perches points out that the subsequent 'seq_putc()' should also be
removed, because the commit that caused all these problems already added
the final '\n' to the seq_printf() above it.
Reported-by: Joe Perches <joe@perches.com>
Fixes: 886c812165 ("mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc")
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Pull PCI fixes from Bjorn Helgaas:
- Fix a tegra enumeration regression (Rob Herring)
- Fix a designware-host check that warned on *success*, not failure
(Alexander Lobakin)
* tag 'pci-v5.11-fixes-1' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci:
PCI: dwc: Fix inverted condition of DMA mask setup warning
PCI: tegra: Fix host link initialization
Clang errors:
drivers/mfd/ab8500-debugfs.c:1526:2: error: non-void function does not return a value [-Werror,-Wreturn-type]
}
^
drivers/mfd/ab8500-debugfs.c:1528:2: error: expected identifier or '('
return 0;
^
drivers/mfd/ab8500-debugfs.c:1529:1: error: extraneous closing brace ('}')
}
^
3 errors generated.
The cleanup in ab8500_interrupts_show left a curly brace around, remove
it to fix the error.
Fixes: 886c812165 ("mfd: ab8500-debugfs: Remove the racy fiddling with irq_desc")
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Commit 660c486590 ("PCI: dwc: Set 32-bit DMA mask for MSI target address
allocation") added dma_mask_set() call to explicitly set 32-bit DMA mask
for MSI message mapping, but for now it throws a warning on ret == 0, while
dma_set_mask() returns 0 in case of success.
Fix this by inverting the condition.
[bhelgaas: join string to make it greppable]
Fixes: 660c486590 ("PCI: dwc: Set 32-bit DMA mask for MSI target address allocation")
Link: https://lore.kernel.org/r/20201222150708.67983-1-alobakin@pm.me
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
