All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	Felipe Balbi <balbi@kernel.org>,
	Eddie Hung <eddie.hung@mediatek.com>
Subject: Re: [PATCH 1/5] usb: mtu3: fix coverity of string buffer
Date: Fri, 8 Jul 2022 09:26:13 +0200	[thread overview]
Message-ID: <YsfcFXtkDxe6ndFT@kroah.com> (raw)
In-Reply-To: <20220708071903.25752-1-chunfeng.yun@mediatek.com>

On Fri, Jul 08, 2022 at 03:18:59PM +0800, Chunfeng Yun wrote:
> Use snprintf instead of sprintf which could cause buffer overflow.

How can it cause an overflow?

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/mtu3/mtu3.h         | 4 +++-
>  drivers/usb/mtu3/mtu3_debugfs.c | 2 +-
>  drivers/usb/mtu3/mtu3_gadget.c  | 4 ++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
> index 8408e1b1a24a..9893dd1bafbb 100644
> --- a/drivers/usb/mtu3/mtu3.h
> +++ b/drivers/usb/mtu3/mtu3.h
> @@ -92,6 +92,8 @@ struct mtu3_request;
>  
>  #define BULK_CLKS_CNT	4
>  
> +#define MTU3_EP_NAME_LEN	12
> +
>  /* device operated link and speed got from DEVICE_CONF register */
>  enum mtu3_speed {
>  	MTU3_SPEED_INACTIVE = 0,
> @@ -272,7 +274,7 @@ struct ssusb_mtk {
>   */
>  struct mtu3_ep {
>  	struct usb_ep ep;
> -	char name[12];
> +	char name[MTU3_EP_NAME_LEN];
>  	struct mtu3 *mtu;
>  	u8 epnum;
>  	u8 type;
> diff --git a/drivers/usb/mtu3/mtu3_debugfs.c b/drivers/usb/mtu3/mtu3_debugfs.c
> index d27de647c86a..a6f72494b819 100644
> --- a/drivers/usb/mtu3/mtu3_debugfs.c
> +++ b/drivers/usb/mtu3/mtu3_debugfs.c
> @@ -132,7 +132,7 @@ static void mtu3_debugfs_regset(struct mtu3 *mtu, void __iomem *base,
>  	if (!mregs)
>  		return;
>  
> -	sprintf(mregs->name, "%s", name);
> +	snprintf(mregs->name, MTU3_DEBUGFS_NAME_LEN, "%s", name);

Where does name come from?  It looks like you control this string, so
there is no overflow anywhere.

>  	regset = &mregs->regset;
>  	regset->regs = regs;
>  	regset->nregs = nregs;
> diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
> index 30999b4debb8..a751e0533c2d 100644
> --- a/drivers/usb/mtu3/mtu3_gadget.c
> +++ b/drivers/usb/mtu3/mtu3_gadget.c
> @@ -635,8 +635,8 @@ static void init_hw_ep(struct mtu3 *mtu, struct mtu3_ep *mep,
>  
>  	INIT_LIST_HEAD(&mep->req_list);
>  
> -	sprintf(mep->name, "ep%d%s", epnum,
> -		!epnum ? "" : (is_in ? "in" : "out"));
> +	snprintf(mep->name, MTU3_EP_NAME_LEN, "ep%d%s", epnum,
> +		 !epnum ? "" : (is_in ? "in" : "out"));

Same here, you already control this string size, so where is the issue?

thanks,

greg k-h


WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	Felipe Balbi <balbi@kernel.org>,
	Eddie Hung <eddie.hung@mediatek.com>
Subject: Re: [PATCH 1/5] usb: mtu3: fix coverity of string buffer
Date: Fri, 8 Jul 2022 09:26:13 +0200	[thread overview]
Message-ID: <YsfcFXtkDxe6ndFT@kroah.com> (raw)
In-Reply-To: <20220708071903.25752-1-chunfeng.yun@mediatek.com>

On Fri, Jul 08, 2022 at 03:18:59PM +0800, Chunfeng Yun wrote:
> Use snprintf instead of sprintf which could cause buffer overflow.

How can it cause an overflow?

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/mtu3/mtu3.h         | 4 +++-
>  drivers/usb/mtu3/mtu3_debugfs.c | 2 +-
>  drivers/usb/mtu3/mtu3_gadget.c  | 4 ++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
> index 8408e1b1a24a..9893dd1bafbb 100644
> --- a/drivers/usb/mtu3/mtu3.h
> +++ b/drivers/usb/mtu3/mtu3.h
> @@ -92,6 +92,8 @@ struct mtu3_request;
>  
>  #define BULK_CLKS_CNT	4
>  
> +#define MTU3_EP_NAME_LEN	12
> +
>  /* device operated link and speed got from DEVICE_CONF register */
>  enum mtu3_speed {
>  	MTU3_SPEED_INACTIVE = 0,
> @@ -272,7 +274,7 @@ struct ssusb_mtk {
>   */
>  struct mtu3_ep {
>  	struct usb_ep ep;
> -	char name[12];
> +	char name[MTU3_EP_NAME_LEN];
>  	struct mtu3 *mtu;
>  	u8 epnum;
>  	u8 type;
> diff --git a/drivers/usb/mtu3/mtu3_debugfs.c b/drivers/usb/mtu3/mtu3_debugfs.c
> index d27de647c86a..a6f72494b819 100644
> --- a/drivers/usb/mtu3/mtu3_debugfs.c
> +++ b/drivers/usb/mtu3/mtu3_debugfs.c
> @@ -132,7 +132,7 @@ static void mtu3_debugfs_regset(struct mtu3 *mtu, void __iomem *base,
>  	if (!mregs)
>  		return;
>  
> -	sprintf(mregs->name, "%s", name);
> +	snprintf(mregs->name, MTU3_DEBUGFS_NAME_LEN, "%s", name);

Where does name come from?  It looks like you control this string, so
there is no overflow anywhere.

>  	regset = &mregs->regset;
>  	regset->regs = regs;
>  	regset->nregs = nregs;
> diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
> index 30999b4debb8..a751e0533c2d 100644
> --- a/drivers/usb/mtu3/mtu3_gadget.c
> +++ b/drivers/usb/mtu3/mtu3_gadget.c
> @@ -635,8 +635,8 @@ static void init_hw_ep(struct mtu3 *mtu, struct mtu3_ep *mep,
>  
>  	INIT_LIST_HEAD(&mep->req_list);
>  
> -	sprintf(mep->name, "ep%d%s", epnum,
> -		!epnum ? "" : (is_in ? "in" : "out"));
> +	snprintf(mep->name, MTU3_EP_NAME_LEN, "ep%d%s", epnum,
> +		 !epnum ? "" : (is_in ? "in" : "out"));

Same here, you already control this string size, so where is the issue?

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2022-07-08  7:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-08  7:18 [PATCH 1/5] usb: mtu3: fix coverity of string buffer Chunfeng Yun
2022-07-08  7:18 ` Chunfeng Yun
2022-07-08  7:19 ` [PATCH 2/5] usb: mtu3: print endpoint type as string Chunfeng Yun
2022-07-08  7:19   ` Chunfeng Yun
2022-07-08  7:19 ` [PATCH 3/5] usb: mtu3: add feature to disable device's usb3 port Chunfeng Yun
2022-07-08  7:19   ` Chunfeng Yun
2022-07-08  7:19 ` [PATCH 4/5] usb: mtu3: check capability of usb3 dual role Chunfeng Yun
2022-07-08  7:19   ` Chunfeng Yun
2022-07-08  7:19 ` [PATCH 5/5] usb: mtu3: support function remote wakeup Chunfeng Yun
2022-07-08  7:19   ` Chunfeng Yun
2022-07-08  7:26 ` Greg Kroah-Hartman [this message]
2022-07-08  7:26   ` [PATCH 1/5] usb: mtu3: fix coverity of string buffer Greg Kroah-Hartman
2022-07-11  6:39   ` Chunfeng Yun
2022-07-11  6:39     ` Chunfeng Yun
2022-07-11  6:45     ` Greg Kroah-Hartman
2022-07-11  6:45       ` Greg Kroah-Hartman
2022-07-13  9:42       ` Chunfeng Yun
2022-07-13  9:42         ` Chunfeng Yun

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=YsfcFXtkDxe6ndFT@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=balbi@kernel.org \
    --cc=chunfeng.yun@mediatek.com \
    --cc=eddie.hung@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    /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.