All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Brosseau, Yannick" <yannick.brosseau@gmail.com>
To: Arnout Vandecappelle <arnout@mind.be>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/openocd: Fix segfault when using ST-Link driver
Date: Wed, 4 May 2022 16:40:24 -0400	[thread overview]
Message-ID: <CAEzWAeLcsFN2t5e5BiDLVAUP9+R6Hefn-XnNdNO3icNxSQO-9Q@mail.gmail.com> (raw)
In-Reply-To: <c9105461-815a-c212-2afc-4c175467b424@mind.be>


[-- Attachment #1.1: Type: text/plain, Size: 6030 bytes --]

On Wed, May 4, 2022 at 3:36 PM Arnout Vandecappelle <arnout@mind.be> wrote:

>
>
> On 04/05/2022 18:42, Yannick Brosseau wrote:
> > Recent changes in libusb have exposed a bug in OpenOCD which now crash
> when trying
> > to use the ST-Link driver.
> >
> > Upstream has a fix as commit cff0e417da58adef1ceef9a63a99412c2cc87ff3.
> This add the commit
> > as a stand alone patch.
> >
> > Should be removed when OpenOCD is updated to a newer release than 0.11
>
> Add a Signed-off-by line for yourself.  This is a short way for you to
> assert that you are entitled to contribute the patch under buildroot's
> GPL license.  See  http://elinux.org/Developer_Certificate_Of_Origin
> for more details.
>
>
I forgot about this one.


> > ---
> >   ...GV-with-libusb-v1.0.24-33-g32a2206-1.patch | 88 +++++++++++++++++++
> >   1 file changed, 88 insertions(+)
> >   create mode 100644
> package/openocd/0003-stlink-fix-SIGSEGV-with-libusb-v1.0.24-33-g32a2206-1.patch
> >
> > diff --git
> a/package/openocd/0003-stlink-fix-SIGSEGV-with-libusb-v1.0.24-33-g32a2206-1.patch
> b/package/openocd/0003-stlink-fix-SIGSEGV-with-libusb-v1.0.24-33-g32a2206-1.patch
> > new file mode 100644
> > index 0000000000..5de3c91fbe
> > --- /dev/null
> > +++
> b/package/openocd/0003-stlink-fix-SIGSEGV-with-libusb-v1.0.24-33-g32a2206-1.patch
> > @@ -0,0 +1,88 @@
> > +From cff0e417da58adef1ceef9a63a99412c2cc87ff3 Mon Sep 17 00:00:00 2001
> > +From: Antonio Borneo <borneo.antonio@gmail.com>
> > +Date: Wed, 23 Jun 2021 16:52:16 +0200
> > +Subject: [PATCH] stlink: fix SIGSEGV with libusb v1.0.24-33-g32a2206
> (11618)
> > +MIME-Version: 1.0
> > +Content-Type: text/plain; charset=UTF-8
> > +Content-Transfer-Encoding: 8bit
> > +
> > +The stlink driver incorrectly uses a NULL pointer for libusb's
> > +struct libusb_context.
> > +The correct value to be used is local in libusb_helper.c.
> > +
> > +Move in the helper file, in a wrapper function, the only call that
> > +requires the above value, and let stlink driver to use this
> > +wrapper.
> > +
> > +This issue has not triggered any visible problem until a code
> > +refactoring [1] in libusb has made OpenOCD crashing on Windows and
> > +on MacOS.
>
>   I guess not only Windows and MacOS but also Linux, otherwise you
> wouldn't have
> sent this, right?
>

Indeed, was also failing on Linux. I'll add a note to make it clear.


>
> > +
> > +Change-Id: Id1818c8af7cf0d4d17dfa1d22aad079da01ef740
> > +Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
> > +Fixes: https://sourceforge.net/p/openocd/tickets/308/
> > +Fixes: https://github.com/libusb/libusb/issues/928/
> > +Fixes: 42d8fa899c6a ("stlink_usb: Submit multiple USB URBs at once to
> improve performance")
> > +Link: [1] https://github.com/libusb/libusb/commit/32a22069428c
> > +Reported-by: Andrzej Sierżęga <asier70@gmail.com>
> > +Co-developed-by: Andrzej Sierżęga <asier70@gmail.com>
> > +Co-developed-by: Xiaofan Chen <xiaofanc@gmail.com>
> > +Reviewed-on: http://openocd.zylin.com/6331
> > +Tested-by: jenkins
> > +Reviewed-by: Marc Schink <dev@zapb.de>
> > +Reviewed-by: Xiaofan <xiaofanc@gmail.com>
> > +Reviewed-by: Andrzej Sierżęga <asier70@gmail.com>
> > +Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>
> > +Reviewed-by: Andreas Fritiofson <andreas.fritiofson@gmail.com>
>
>   Here, you need to add your Signed-off-by as well. This is needed because
> the
> package license may be different from Buildroot's, and the patch takes the
> package's license.
>
>
But did not know about that one. Interesting.

Will update and resend.

Yannick



>   Regards,
>   Arnout
>
> > +---
> > + src/jtag/drivers/libusb_helper.c | 5 +++++
> > + src/jtag/drivers/libusb_helper.h | 1 +
> > + src/jtag/drivers/stlink_usb.c    | 7 +------
> > + 3 files changed, 7 insertions(+), 6 deletions(-)
> > +
> > +diff --git a/src/jtag/drivers/libusb_helper.c
> b/src/jtag/drivers/libusb_helper.c
> > +index f0122d534..18fe4bad4 100644
> > +--- a/src/jtag/drivers/libusb_helper.c
> > ++++ b/src/jtag/drivers/libusb_helper.c
> > +@@ -363,3 +363,8 @@ int jtag_libusb_get_pid(struct libusb_device *dev,
> uint16_t *pid)
> > +
> > +     return ERROR_FAIL;
> > + }
> > ++
> > ++int jtag_libusb_handle_events_completed(int *completed)
> > ++{
> > ++    return libusb_handle_events_completed(jtag_libusb_context,
> completed);
> > ++}
> > +diff --git a/src/jtag/drivers/libusb_helper.h
> b/src/jtag/drivers/libusb_helper.h
> > +index fa7d06e28..3e77865d6 100644
> > +--- a/src/jtag/drivers/libusb_helper.h
> > ++++ b/src/jtag/drivers/libusb_helper.h
> > +@@ -60,5 +60,6 @@ int jtag_libusb_choose_interface(struct
> libusb_device_handle *devh,
> > +             unsigned int *usb_write_ep,
> > +             int bclass, int subclass, int protocol, int trans_type);
> > + int jtag_libusb_get_pid(struct libusb_device *dev, uint16_t *pid);
> > ++int jtag_libusb_handle_events_completed(int *completed);
> > +
> > + #endif /* OPENOCD_JTAG_DRIVERS_LIBUSB_HELPER_H */
> > +diff --git a/src/jtag/drivers/stlink_usb.c
> b/src/jtag/drivers/stlink_usb.c
> > +index c68bbb3ca..7b1932b9f 100644
> > +--- a/src/jtag/drivers/stlink_usb.c
> > ++++ b/src/jtag/drivers/stlink_usb.c
> > +@@ -497,13 +497,8 @@ static void
> sync_transfer_wait_for_completion(struct libusb_transfer *transfer)
> > + {
> > +     int r, *completed = transfer->user_data;
> > +
> > +-    /* Assuming a single libusb context exists.  There no existing
> interface into this
> > +-     * module to pass a libusb context.
> > +-     */
> > +-    struct libusb_context *ctx = NULL;
> > +-
> > +     while (!*completed) {
> > +-            r = libusb_handle_events_completed(ctx, completed);
> > ++            r = jtag_libusb_handle_events_completed(completed);
> > +             if (r < 0) {
> > +                     if (r == LIBUSB_ERROR_INTERRUPTED)
> > +                             continue;
> > +--
> > +2.35.1
> > +
>

[-- Attachment #1.2: Type: text/html, Size: 8646 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

      reply	other threads:[~2022-05-04 20:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-04 16:42 [Buildroot] [PATCH] package/openocd: Fix segfault when using ST-Link driver Yannick Brosseau
2022-05-04 19:36 ` Arnout Vandecappelle
2022-05-04 20:40   ` Brosseau, Yannick [this message]

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=CAEzWAeLcsFN2t5e5BiDLVAUP9+R6Hefn-XnNdNO3icNxSQO-9Q@mail.gmail.com \
    --to=yannick.brosseau@gmail.com \
    --cc=arnout@mind.be \
    --cc=buildroot@buildroot.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.