| Message ID | 20241231161539.20192-3-xndchn@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usbip: Fix seqnum sign extension and format specifier issues | expand |
On 12/31/24 09:15, Xiong Nandi wrote: > The seqnum field in USBIP protocol is an unsigned value. > So we fix the format specifier from %d to %u to correctly > display the value. > How did you find the problem? Include log from the tool or compiler output. > Signed-off-by: Xiong Nandi <xndchn@gmail.com> > --- > drivers/usb/usbip/stub_rx.c | 2 +- > drivers/usb/usbip/stub_tx.c | 2 +- > drivers/usb/usbip/vhci_rx.c | 6 +++--- > drivers/usb/usbip/vudc_tx.c | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c > index 6338d818bc8b..9aa30ef76f3b 100644 > --- a/drivers/usb/usbip/stub_rx.c > +++ b/drivers/usb/usbip/stub_rx.c > @@ -269,7 +269,7 @@ static int stub_recv_cmd_unlink(struct stub_device *sdev, > return 0; > } > > - usbip_dbg_stub_rx("seqnum %d is not pending\n", > + usbip_dbg_stub_rx("seqnum %u is not pending\n", > pdu->u.cmd_unlink.seqnum); seqnum is unsigned long - don't you have to use %ul? > > /* > diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c > index b1c2f6781cb3..7eb2e074012a 100644 > --- a/drivers/usb/usbip/stub_tx.c > +++ b/drivers/usb/usbip/stub_tx.c > @@ -201,7 +201,7 @@ static int stub_send_ret_submit(struct stub_device *sdev) > > /* 1. setup usbip_header */ > setup_ret_submit_pdu(&pdu_header, urb); > - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", > + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", > pdu_header.base.seqnum); > > if (priv->sgl) { > diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c > index 7f2d1c241559..a75f4a898a41 100644 > --- a/drivers/usb/usbip/vhci_rx.c > +++ b/drivers/usb/usbip/vhci_rx.c > @@ -66,7 +66,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev, > spin_unlock_irqrestore(&vdev->priv_lock, flags); > > if (!urb) { > - pr_err("cannot find a urb of seqnum %u max seqnum %d\n", > + pr_err("cannot find a urb of seqnum %u max seqnum %u\n", > pdu->base.seqnum, > atomic_read(&vhci_hcd->seqnum)); > usbip_event_add(ud, VDEV_EVENT_ERROR_TCP); > @@ -162,10 +162,10 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev, > * already received the result of its submit result and gave > * back the URB. > */ > - pr_info("the urb (seqnum %d) was already given back\n", > + pr_info("the urb (seqnum %u) was already given back\n", > pdu->base.seqnum); > } else { > - usbip_dbg_vhci_rx("now giveback urb %d\n", pdu->base.seqnum); > + usbip_dbg_vhci_rx("now giveback urb %u\n", pdu->base.seqnum); > > /* If unlink is successful, status is -ECONNRESET */ > urb->status = pdu->u.ret_unlink.status; > diff --git a/drivers/usb/usbip/vudc_tx.c b/drivers/usb/usbip/vudc_tx.c > index 3ccb17c3e840..30c11bf9f4e7 100644 > --- a/drivers/usb/usbip/vudc_tx.c > +++ b/drivers/usb/usbip/vudc_tx.c > @@ -107,7 +107,7 @@ static int v_send_ret_submit(struct vudc *udc, struct urbp *urb_p) > > /* 1. setup usbip_header */ > setup_ret_submit_pdu(&pdu_header, urb_p); > - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", > + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", > pdu_header.base.seqnum); > usbip_header_correct_endian(&pdu_header, 1); > thanks, -- Shuah
Thanks. > How did you find the problem? Include log from the tool > or compiler output. As said in [Patch 1/2], I saw this log: > [ 294.204334] vhci_hcd: cannot find a urb of seqnum 2147483648 max seqnum -2147483648 then I found there are also logs with wrong format specifier for seqnum > seqnum is unsigned long - don't you have to use %ul? pdu->u.cmd_unlink.seqnum is defined as u32 actually. In usbip driver, all seqnum in protocol struct (struct usbip_header_basic and struct usbip_header_cmd_unlink), is defined as u32. In other driver specific priv struct, seqnum is defined as unsigned long. It seems only the u32 seqnum have wrong format specifier On Fri, Jan 3, 2025 at 6:09 AM Shuah Khan <skhan@linuxfoundation.org> wrote: > > On 12/31/24 09:15, Xiong Nandi wrote: > > The seqnum field in USBIP protocol is an unsigned value. > > So we fix the format specifier from %d to %u to correctly > > display the value. > > > > How did you find the problem? Include log from the tool > or compiler output. > > > Signed-off-by: Xiong Nandi <xndchn@gmail.com> > > --- > > drivers/usb/usbip/stub_rx.c | 2 +- > > drivers/usb/usbip/stub_tx.c | 2 +- > > drivers/usb/usbip/vhci_rx.c | 6 +++--- > > drivers/usb/usbip/vudc_tx.c | 2 +- > > 4 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c > > index 6338d818bc8b..9aa30ef76f3b 100644 > > --- a/drivers/usb/usbip/stub_rx.c > > +++ b/drivers/usb/usbip/stub_rx.c > > @@ -269,7 +269,7 @@ static int stub_recv_cmd_unlink(struct stub_device *sdev, > > return 0; > > } > > > > - usbip_dbg_stub_rx("seqnum %d is not pending\n", > > + usbip_dbg_stub_rx("seqnum %u is not pending\n", > > pdu->u.cmd_unlink.seqnum); > > seqnum is unsigned long - don't you have to use %ul? > > > > /* > > diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c > > index b1c2f6781cb3..7eb2e074012a 100644 > > --- a/drivers/usb/usbip/stub_tx.c > > +++ b/drivers/usb/usbip/stub_tx.c > > @@ -201,7 +201,7 @@ static int stub_send_ret_submit(struct stub_device *sdev) > > > > /* 1. setup usbip_header */ > > setup_ret_submit_pdu(&pdu_header, urb); > > - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", > > + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", > > pdu_header.base.seqnum); > > > > if (priv->sgl) { > > diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c > > index 7f2d1c241559..a75f4a898a41 100644 > > --- a/drivers/usb/usbip/vhci_rx.c > > +++ b/drivers/usb/usbip/vhci_rx.c > > @@ -66,7 +66,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev, > > spin_unlock_irqrestore(&vdev->priv_lock, flags); > > > > if (!urb) { > > - pr_err("cannot find a urb of seqnum %u max seqnum %d\n", > > + pr_err("cannot find a urb of seqnum %u max seqnum %u\n", > > pdu->base.seqnum, > > atomic_read(&vhci_hcd->seqnum)); > > usbip_event_add(ud, VDEV_EVENT_ERROR_TCP); > > @@ -162,10 +162,10 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev, > > * already received the result of its submit result and gave > > * back the URB. > > */ > > - pr_info("the urb (seqnum %d) was already given back\n", > > + pr_info("the urb (seqnum %u) was already given back\n", > > pdu->base.seqnum); > > } else { > > - usbip_dbg_vhci_rx("now giveback urb %d\n", pdu->base.seqnum); > > + usbip_dbg_vhci_rx("now giveback urb %u\n", pdu->base.seqnum); > > > > /* If unlink is successful, status is -ECONNRESET */ > > urb->status = pdu->u.ret_unlink.status; > > diff --git a/drivers/usb/usbip/vudc_tx.c b/drivers/usb/usbip/vudc_tx.c > > index 3ccb17c3e840..30c11bf9f4e7 100644 > > --- a/drivers/usb/usbip/vudc_tx.c > > +++ b/drivers/usb/usbip/vudc_tx.c > > @@ -107,7 +107,7 @@ static int v_send_ret_submit(struct vudc *udc, struct urbp *urb_p) > > > > /* 1. setup usbip_header */ > > setup_ret_submit_pdu(&pdu_header, urb_p); > > - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", > > + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", > > pdu_header.base.seqnum); > > usbip_header_correct_endian(&pdu_header, 1); > > > thanks, > -- Shuah
diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c index 6338d818bc8b..9aa30ef76f3b 100644 --- a/drivers/usb/usbip/stub_rx.c +++ b/drivers/usb/usbip/stub_rx.c @@ -269,7 +269,7 @@ static int stub_recv_cmd_unlink(struct stub_device *sdev, return 0; } - usbip_dbg_stub_rx("seqnum %d is not pending\n", + usbip_dbg_stub_rx("seqnum %u is not pending\n", pdu->u.cmd_unlink.seqnum); /* diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c index b1c2f6781cb3..7eb2e074012a 100644 --- a/drivers/usb/usbip/stub_tx.c +++ b/drivers/usb/usbip/stub_tx.c @@ -201,7 +201,7 @@ static int stub_send_ret_submit(struct stub_device *sdev) /* 1. setup usbip_header */ setup_ret_submit_pdu(&pdu_header, urb); - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", pdu_header.base.seqnum); if (priv->sgl) { diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c index 7f2d1c241559..a75f4a898a41 100644 --- a/drivers/usb/usbip/vhci_rx.c +++ b/drivers/usb/usbip/vhci_rx.c @@ -66,7 +66,7 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev, spin_unlock_irqrestore(&vdev->priv_lock, flags); if (!urb) { - pr_err("cannot find a urb of seqnum %u max seqnum %d\n", + pr_err("cannot find a urb of seqnum %u max seqnum %u\n", pdu->base.seqnum, atomic_read(&vhci_hcd->seqnum)); usbip_event_add(ud, VDEV_EVENT_ERROR_TCP); @@ -162,10 +162,10 @@ static void vhci_recv_ret_unlink(struct vhci_device *vdev, * already received the result of its submit result and gave * back the URB. */ - pr_info("the urb (seqnum %d) was already given back\n", + pr_info("the urb (seqnum %u) was already given back\n", pdu->base.seqnum); } else { - usbip_dbg_vhci_rx("now giveback urb %d\n", pdu->base.seqnum); + usbip_dbg_vhci_rx("now giveback urb %u\n", pdu->base.seqnum); /* If unlink is successful, status is -ECONNRESET */ urb->status = pdu->u.ret_unlink.status; diff --git a/drivers/usb/usbip/vudc_tx.c b/drivers/usb/usbip/vudc_tx.c index 3ccb17c3e840..30c11bf9f4e7 100644 --- a/drivers/usb/usbip/vudc_tx.c +++ b/drivers/usb/usbip/vudc_tx.c @@ -107,7 +107,7 @@ static int v_send_ret_submit(struct vudc *udc, struct urbp *urb_p) /* 1. setup usbip_header */ setup_ret_submit_pdu(&pdu_header, urb_p); - usbip_dbg_stub_tx("setup txdata seqnum: %d\n", + usbip_dbg_stub_tx("setup txdata seqnum: %u\n", pdu_header.base.seqnum); usbip_header_correct_endian(&pdu_header, 1);
The seqnum field in USBIP protocol is an unsigned value. So we fix the format specifier from %d to %u to correctly display the value. Signed-off-by: Xiong Nandi <xndchn@gmail.com> --- drivers/usb/usbip/stub_rx.c | 2 +- drivers/usb/usbip/stub_tx.c | 2 +- drivers/usb/usbip/vhci_rx.c | 6 +++--- drivers/usb/usbip/vudc_tx.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-)