* [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() @ 2019-01-30 5:39 Myungho Jung 2019-01-31 15:40 ` Johan Hovold 0 siblings, 1 reply; 8+ messages in thread From: Myungho Jung @ 2019-01-30 5:39 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg; +Cc: linux-bluetooth, linux-kernel tiocmget() and tiocmset() operations are optional and some tty drivers like pty miss the operations. We need NULL check to prevent from dereference. Signed-off-by: Myungho Jung <mhjungk@gmail.com> --- drivers/bluetooth/hci_ath.c | 6 ++++++ drivers/bluetooth/hci_ldisc.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c index d568fbd94d6c..fb9f6323a911 100644 --- a/drivers/bluetooth/hci_ath.c +++ b/drivers/bluetooth/hci_ath.c @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) static int ath_setup(struct hci_uart *hu) { + struct tty_struct *tty = hu->tty; + BT_DBG("hu %p", hu); + /* tty driver should support operations to set RTS */ + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) + return -EOPNOTSUPP; + hu->hdev->set_bdaddr = ath_set_bdaddr; return 0; diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index fbf7b4df23ab..cb31c2d8d826 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -314,6 +314,10 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable) return; } + /* tiocmget() and tiocmset() operations are optional */ + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) + return; + if (enable) { /* Disable hardware flow control */ ktermios = tty->termios; -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-01-30 5:39 [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() Myungho Jung @ 2019-01-31 15:40 ` Johan Hovold 2019-02-03 6:38 ` Myungho Jung 0 siblings, 1 reply; 8+ messages in thread From: Johan Hovold @ 2019-01-31 15:40 UTC (permalink / raw) To: Myungho Jung Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Tue, Jan 29, 2019 at 09:39:28PM -0800, Myungho Jung wrote: > tiocmget() and tiocmset() operations are optional and some tty drivers > like pty miss the operations. We need NULL check to prevent from > dereference. > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > --- > drivers/bluetooth/hci_ath.c | 6 ++++++ > drivers/bluetooth/hci_ldisc.c | 4 ++++ > 2 files changed, 10 insertions(+) Ah, you had already submitted a v2. I still suggest splitting this one in two patches and that you add a Fixes and stable tag to each so that they both get backported to stable. Also, when resubmitting, make sure to include a short changelog here below the cut-off line (---). > > diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c > index d568fbd94d6c..fb9f6323a911 100644 > --- a/drivers/bluetooth/hci_ath.c > +++ b/drivers/bluetooth/hci_ath.c > @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) > > static int ath_setup(struct hci_uart *hu) > { > + struct tty_struct *tty = hu->tty; > + > BT_DBG("hu %p", hu); > > + /* tty driver should support operations to set RTS */ > + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) > + return -EOPNOTSUPP; -ENODEV might be more appropriate. Johan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-01-31 15:40 ` Johan Hovold @ 2019-02-03 6:38 ` Myungho Jung 2019-02-03 10:53 ` Johan Hovold 0 siblings, 1 reply; 8+ messages in thread From: Myungho Jung @ 2019-02-03 6:38 UTC (permalink / raw) To: Johan Hovold Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Thu, Jan 31, 2019 at 04:40:00PM +0100, Johan Hovold wrote: > On Tue, Jan 29, 2019 at 09:39:28PM -0800, Myungho Jung wrote: > > tiocmget() and tiocmset() operations are optional and some tty drivers > > like pty miss the operations. We need NULL check to prevent from > > dereference. > > > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > --- > > drivers/bluetooth/hci_ath.c | 6 ++++++ > > drivers/bluetooth/hci_ldisc.c | 4 ++++ > > 2 files changed, 10 insertions(+) > > Ah, you had already submitted a v2. > > I still suggest splitting this one in two patches and that you add a > Fixes and stable tag to each so that they both get backported to stable. > > Also, when resubmitting, make sure to include a short changelog here > below the cut-off line (---). > > > > > diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c > > index d568fbd94d6c..fb9f6323a911 100644 > > --- a/drivers/bluetooth/hci_ath.c > > +++ b/drivers/bluetooth/hci_ath.c > > @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) > > > > static int ath_setup(struct hci_uart *hu) > > { > > + struct tty_struct *tty = hu->tty; > > + > > BT_DBG("hu %p", hu); > > > > + /* tty driver should support operations to set RTS */ > > + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) > > + return -EOPNOTSUPP; > > -ENODEV might be more appropriate. > > Johan I'll split into 2 seperate patches. So, do I need to add stable tag on each patch like below to be cherry-picked? Cc: <stable@vger.kernel.org> # <hash>: <summary> I looked for a good example from mailing list but didn't find one. Thanks, Myungho ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-02-03 6:38 ` Myungho Jung @ 2019-02-03 10:53 ` Johan Hovold 2019-02-04 7:29 ` Myungho Jung 0 siblings, 1 reply; 8+ messages in thread From: Johan Hovold @ 2019-02-03 10:53 UTC (permalink / raw) To: Myungho Jung Cc: Johan Hovold, Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Sat, Feb 02, 2019 at 10:38:24PM -0800, Myungho Jung wrote: > On Thu, Jan 31, 2019 at 04:40:00PM +0100, Johan Hovold wrote: > > On Tue, Jan 29, 2019 at 09:39:28PM -0800, Myungho Jung wrote: > > > tiocmget() and tiocmset() operations are optional and some tty drivers > > > like pty miss the operations. We need NULL check to prevent from > > > dereference. > > > > > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > > --- > > > drivers/bluetooth/hci_ath.c | 6 ++++++ > > > drivers/bluetooth/hci_ldisc.c | 4 ++++ > > > 2 files changed, 10 insertions(+) > > > > Ah, you had already submitted a v2. > > > > I still suggest splitting this one in two patches and that you add a > > Fixes and stable tag to each so that they both get backported to stable. > > > > Also, when resubmitting, make sure to include a short changelog here > > below the cut-off line (---). > > > > > > > > diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c > > > index d568fbd94d6c..fb9f6323a911 100644 > > > --- a/drivers/bluetooth/hci_ath.c > > > +++ b/drivers/bluetooth/hci_ath.c > > > @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) > > > > > > static int ath_setup(struct hci_uart *hu) > > > { > > > + struct tty_struct *tty = hu->tty; > > > + > > > BT_DBG("hu %p", hu); > > > > > > + /* tty driver should support operations to set RTS */ > > > + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) > > > + return -EOPNOTSUPP; > > > > -ENODEV might be more appropriate. > > > > Johan > > I'll split into 2 seperate patches. So, do I need to add stable tag on each > patch like below to be cherry-picked? > > Cc: <stable@vger.kernel.org> # <hash>: <summary> > > I looked for a good example from mailing list but didn't find one. Almost right, the format you use above is actually used to identify dependencies for backports. You should add a Fixes tag identifying the commit which introduced each bug and a stable-cc tag. If you want you can indicate the version after a # sign, but that can also be inferred from the fixes tag. For the hci_ldisc fix I think the tags would be: Fixes: 2a973dfada2b ("Bluetooth: hci_uart: Add new line discipline enhancements") Cc: stable <stable@vger.kernel.org> # 4.2 You can use git blame to track down the offending commits. This should all be explained here: https://www.kernel.org/doc/html/latest/process/submitting-patches.html Johan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-02-03 10:53 ` Johan Hovold @ 2019-02-04 7:29 ` Myungho Jung 2019-02-04 9:04 ` Myungho Jung 0 siblings, 1 reply; 8+ messages in thread From: Myungho Jung @ 2019-02-04 7:29 UTC (permalink / raw) To: Johan Hovold Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Sun, Feb 03, 2019 at 11:53:23AM +0100, Johan Hovold wrote: > On Sat, Feb 02, 2019 at 10:38:24PM -0800, Myungho Jung wrote: > > On Thu, Jan 31, 2019 at 04:40:00PM +0100, Johan Hovold wrote: > > > On Tue, Jan 29, 2019 at 09:39:28PM -0800, Myungho Jung wrote: > > > > tiocmget() and tiocmset() operations are optional and some tty drivers > > > > like pty miss the operations. We need NULL check to prevent from > > > > dereference. > > > > > > > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > > > --- > > > > drivers/bluetooth/hci_ath.c | 6 ++++++ > > > > drivers/bluetooth/hci_ldisc.c | 4 ++++ > > > > 2 files changed, 10 insertions(+) > > > > > > Ah, you had already submitted a v2. > > > > > > I still suggest splitting this one in two patches and that you add a > > > Fixes and stable tag to each so that they both get backported to stable. > > > > > > Also, when resubmitting, make sure to include a short changelog here > > > below the cut-off line (---). > > > > > > > > > > > diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c > > > > index d568fbd94d6c..fb9f6323a911 100644 > > > > --- a/drivers/bluetooth/hci_ath.c > > > > +++ b/drivers/bluetooth/hci_ath.c > > > > @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) > > > > > > > > static int ath_setup(struct hci_uart *hu) > > > > { > > > > + struct tty_struct *tty = hu->tty; > > > > + > > > > BT_DBG("hu %p", hu); > > > > > > > > + /* tty driver should support operations to set RTS */ > > > > + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) > > > > + return -EOPNOTSUPP; > > > > > > -ENODEV might be more appropriate. > > > > > > Johan > > > > I'll split into 2 seperate patches. So, do I need to add stable tag on each > > patch like below to be cherry-picked? > > > > Cc: <stable@vger.kernel.org> # <hash>: <summary> > > > > I looked for a good example from mailing list but didn't find one. > > Almost right, the format you use above is actually used to identify > dependencies for backports. > > You should add a Fixes tag identifying the commit which introduced each > bug and a stable-cc tag. If you want you can indicate the version after > a # sign, but that can also be inferred from the fixes tag. > > For the hci_ldisc fix I think the tags would be: > > Fixes: 2a973dfada2b ("Bluetooth: hci_uart: Add new line discipline enhancements") > Cc: stable <stable@vger.kernel.org> # 4.2 > > You can use git blame to track down the offending commits. > > This should all be explained here: > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > Johan Thank you for the explanation. I'll carefully read the page and submit the next patch. Thanks, Myungho ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-02-04 7:29 ` Myungho Jung @ 2019-02-04 9:04 ` Myungho Jung 2019-02-04 9:22 ` Johan Hovold 0 siblings, 1 reply; 8+ messages in thread From: Myungho Jung @ 2019-02-04 9:04 UTC (permalink / raw) To: Johan Hovold Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Sun, Feb 03, 2019 at 11:29:00PM -0800, Myungho Jung wrote: > On Sun, Feb 03, 2019 at 11:53:23AM +0100, Johan Hovold wrote: > > On Sat, Feb 02, 2019 at 10:38:24PM -0800, Myungho Jung wrote: > > > On Thu, Jan 31, 2019 at 04:40:00PM +0100, Johan Hovold wrote: > > > > On Tue, Jan 29, 2019 at 09:39:28PM -0800, Myungho Jung wrote: > > > > > tiocmget() and tiocmset() operations are optional and some tty drivers > > > > > like pty miss the operations. We need NULL check to prevent from > > > > > dereference. > > > > > > > > > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > > > > --- > > > > > drivers/bluetooth/hci_ath.c | 6 ++++++ > > > > > drivers/bluetooth/hci_ldisc.c | 4 ++++ > > > > > 2 files changed, 10 insertions(+) > > > > > > > > Ah, you had already submitted a v2. > > > > > > > > I still suggest splitting this one in two patches and that you add a > > > > Fixes and stable tag to each so that they both get backported to stable. > > > > > > > > Also, when resubmitting, make sure to include a short changelog here > > > > below the cut-off line (---). > > > > > > > > > > > > > > diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c > > > > > index d568fbd94d6c..fb9f6323a911 100644 > > > > > --- a/drivers/bluetooth/hci_ath.c > > > > > +++ b/drivers/bluetooth/hci_ath.c > > > > > @@ -185,8 +185,14 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr) > > > > > > > > > > static int ath_setup(struct hci_uart *hu) > > > > > { > > > > > + struct tty_struct *tty = hu->tty; > > > > > + > > > > > BT_DBG("hu %p", hu); > > > > > > > > > > + /* tty driver should support operations to set RTS */ > > > > > + if (!tty->driver->ops->tiocmget || !tty->driver->ops->tiocmset) > > > > > + return -EOPNOTSUPP; > > > > > > > > -ENODEV might be more appropriate. > > > > > > > > Johan > > > > > > I'll split into 2 seperate patches. So, do I need to add stable tag on each > > > patch like below to be cherry-picked? > > > > > > Cc: <stable@vger.kernel.org> # <hash>: <summary> > > > > > > I looked for a good example from mailing list but didn't find one. > > > > Almost right, the format you use above is actually used to identify > > dependencies for backports. > > > > You should add a Fixes tag identifying the commit which introduced each > > bug and a stable-cc tag. If you want you can indicate the version after > > a # sign, but that can also be inferred from the fixes tag. > > > > For the hci_ldisc fix I think the tags would be: > > > > Fixes: 2a973dfada2b ("Bluetooth: hci_uart: Add new line discipline enhancements") > > Cc: stable <stable@vger.kernel.org> # 4.2 > > > > You can use git blame to track down the offending commits. > > > > This should all be explained here: > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > > > Johan > > Thank you for the explanation. I'll carefully read the page and submit the next > patch. > > Thanks, > Myungho Hi Johan, I have one more question. The title of new patches should be version 3 like this? [PATCH v3] Bluetooth: hci_ath: ... [PATCH v3] Bluetooth: hci_ldisc: ... Or newly start with [PATCH]? Thanks, Myungho ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-02-04 9:04 ` Myungho Jung @ 2019-02-04 9:22 ` Johan Hovold 2019-02-04 9:29 ` Myungho Jung 0 siblings, 1 reply; 8+ messages in thread From: Johan Hovold @ 2019-02-04 9:22 UTC (permalink / raw) To: Myungho Jung Cc: Johan Hovold, Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Mon, Feb 04, 2019 at 01:04:37AM -0800, Myungho Jung wrote: > On Sun, Feb 03, 2019 at 11:29:00PM -0800, Myungho Jung wrote: > > On Sun, Feb 03, 2019 at 11:53:23AM +0100, Johan Hovold wrote: > > > You should add a Fixes tag identifying the commit which introduced each > > > bug and a stable-cc tag. If you want you can indicate the version after > > > a # sign, but that can also be inferred from the fixes tag. > > > > > > For the hci_ldisc fix I think the tags would be: > > > > > > Fixes: 2a973dfada2b ("Bluetooth: hci_uart: Add new line discipline enhancements") > > > Cc: stable <stable@vger.kernel.org> # 4.2 > > > > > > You can use git blame to track down the offending commits. > > > > > > This should all be explained here: > > > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > I have one more question. The title of new patches should be version 3 like > this? > [PATCH v3] Bluetooth: hci_ath: ... > [PATCH v3] Bluetooth: hci_ldisc: ... > > Or newly start with [PATCH]? You should keep and increment the revision number even if you split a patch (so use v3 for the resend). I suggest you send both patches in a series (as they are related); take a look at git-send-email for a convenient way to do that. And always include a brief changelog (below the cut-off line, "---") when revising patches. Johan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() 2019-02-04 9:22 ` Johan Hovold @ 2019-02-04 9:29 ` Myungho Jung 0 siblings, 0 replies; 8+ messages in thread From: Myungho Jung @ 2019-02-04 9:29 UTC (permalink / raw) To: Johan Hovold Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel On Mon, Feb 04, 2019 at 10:22:16AM +0100, Johan Hovold wrote: > On Mon, Feb 04, 2019 at 01:04:37AM -0800, Myungho Jung wrote: > > On Sun, Feb 03, 2019 at 11:29:00PM -0800, Myungho Jung wrote: > > > On Sun, Feb 03, 2019 at 11:53:23AM +0100, Johan Hovold wrote: > > > > > You should add a Fixes tag identifying the commit which introduced each > > > > bug and a stable-cc tag. If you want you can indicate the version after > > > > a # sign, but that can also be inferred from the fixes tag. > > > > > > > > For the hci_ldisc fix I think the tags would be: > > > > > > > > Fixes: 2a973dfada2b ("Bluetooth: hci_uart: Add new line discipline enhancements") > > > > Cc: stable <stable@vger.kernel.org> # 4.2 > > > > > > > > You can use git blame to track down the offending commits. > > > > > > > > This should all be explained here: > > > > > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > > I have one more question. The title of new patches should be version 3 like > > this? > > [PATCH v3] Bluetooth: hci_ath: ... > > [PATCH v3] Bluetooth: hci_ldisc: ... > > > > Or newly start with [PATCH]? > > You should keep and increment the revision number even if you split a > patch (so use v3 for the resend). > > I suggest you send both patches in a series (as they are related); take > a look at git-send-email for a convenient way to do that. > > And always include a brief changelog (below the cut-off line, "---") > when revising patches. > > Johan I see. I'll submit version 3 as a patchset with change logs. Thanks, Myungho ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-04 9:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-30 5:39 [PATCH v2] Bluetooth: Add NULL check for tiocmget() and tiocmset() Myungho Jung 2019-01-31 15:40 ` Johan Hovold 2019-02-03 6:38 ` Myungho Jung 2019-02-03 10:53 ` Johan Hovold 2019-02-04 7:29 ` Myungho Jung 2019-02-04 9:04 ` Myungho Jung 2019-02-04 9:22 ` Johan Hovold 2019-02-04 9:29 ` Myungho Jung
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).