From: Stephen Boyd <swboyd@chromium.org>
To: gregkh@linuxfoundation.org, satya priya <skakit@codeaurora.org>
Cc: mgautam@codeaurora.org, linux-arm-msm@vger.kernel.org,
linux-serial@vger.kernel.org, akashast@codeaurora.org,
rojay@codeaurora.org, msavaliy@qti.qualcomm.com,
satya priya <skakit@codeaurora.org>
Subject: Re: [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe
Date: Fri, 28 Feb 2020 15:01:29 -0800 [thread overview]
Message-ID: <158293088963.112031.11417422453396901116@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <1582638862-9344-2-git-send-email-skakit@codeaurora.org>
Quoting satya priya (2020-02-25 05:54:21)
> To fix the RX cancel command failure, rx_fifo buffer needs to be
> flushed in stop_rx() by calling handle_rx().
>
> If set_termios is called before startup, by this time memory is not
> allocated to port->rx_fifo buffer, which leads to a NULL pointer
> dereference.
Also, clearly set_termios() isn't being called in the warning stack that
I sent last round:
pc : handle_rx_uart+0x64/0x278
lr : qcom_geni_serial_handle_rx+0x84/0x90
sp : ffffff814348f960
x29: ffffff814348f960 x28: ffffffd01ac24288
x27: 0000000000000018 x26: 0000000000000002
x25: 0000000000000001 x24: ffffff8146341348
x23: ffffff8146341000 x22: ffffffd01accc978
x21: ffffff8146341000 x20: 0000000000000001
x19: 0000000000000001 x18: ffffffd01b22d000
x17: 0000000000008000 x16: 00000000000000b0
x15: ffffffd01afdbdd0 x14: ffffffd01b3edde0
x13: ffffffd01b7fb000 x12: 0000000000000001
x11: 0000000000000000 x10: 0000000000000000
x9 : ffffffd010344780 x8 : 0000000000000000
x7 : ffffffd019d8e768 x6 : 0000000000000000
x5 : ffffffd01adbb000 x4 : 0000000000008004
x3 : 0000000000000001 x2 : 0000000000000001
x1 : 0000000000000001 x0 : ffffffd01accc978
Call trace:
handle_rx_uart+0x64/0x278
qcom_geni_serial_handle_rx+0x84/0x90
qcom_geni_serial_stop_rx+0x110/0x180
qcom_geni_serial_port_setup+0x68/0x1b0
qcom_geni_serial_startup+0x24/0x70
uart_startup+0x164/0x28c
uart_port_activate+0x6c/0xbc
tty_port_open+0xa8/0x114
uart_open+0x28/0x38
ttyport_open+0x7c/0x164
serdev_device_open+0x38/0xe4
hci_uart_register_device+0x54/0x2e8 [hci_uart]
qca_serdev_probe+0x1c4/0x374 [hci_uart]
serdev_drv_probe+0x3c/0x64
really_probe+0x144/0x3f8
driver_probe_device+0x70/0x140
__driver_attach_async_helper+0x7c/0xa8
async_run_entry_fn+0x60/0x178
process_one_work+0x33c/0x640
worker_thread+0x2a0/0x470
kthread+0x128/0x138
ret_from_fork+0x10/0x18
Code: 1aca096a 911e0129 b940012b 7100054a (b800450b)
This shows that uart_startup() is the one that is calling
qcom_geni_serial_startup() and that's running the newly added cancel
path. So even if we allocate the buffer in probe vs. in startup we're
going to flip a buffer full of junk that we're trying to cancel out of
the fifo into the tty layer. That seems wrong. We should have a
different qcom_geni_serial_stop_rx() function that knows we're starting
up vs. handling a normal rx event and call something besides handle_rx()
because that pushes bytes up into the tty layer.
>
> To avoid this NULL pointer dereference allocate memory to port->rx_fifo
> in probe itself.
>
next prev parent reply other threads:[~2020-02-28 23:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 13:54 [PATCH V2 0/2] Fix RX cancel command failure satya priya
2020-02-25 13:54 ` [PATCH V2 1/2] tty: serial: qcom_geni_serial: Allocate port->rx_fifo buffer in probe satya priya
2020-02-28 22:54 ` Stephen Boyd
2020-03-04 13:38 ` skakit
2020-02-28 23:01 ` Stephen Boyd [this message]
2020-03-04 13:34 ` skakit
2020-03-04 17:48 ` Stephen Boyd
2020-02-25 13:54 ` [PATCH V2 2/2] tty: serial: qcom_geni_serial: Fix RX cancel command failure satya priya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=158293088963.112031.11417422453396901116@swboyd.mtv.corp.google.com \
--to=swboyd@chromium.org \
--cc=akashast@codeaurora.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=mgautam@codeaurora.org \
--cc=msavaliy@qti.qualcomm.com \
--cc=rojay@codeaurora.org \
--cc=skakit@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.