* [PATCH 1/2] usb: typec: tipd: Don't read/write more bytes than required
@ 2021-09-14 10:42 Sven Peter
2021-09-14 10:42 ` [PATCH 2/2] usb: typec: tipd: Add an additional overflow check Sven Peter
0 siblings, 1 reply; 5+ messages in thread
From: Sven Peter @ 2021-09-14 10:42 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Sven Peter
tps6598x_block_read/write always read 65 bytes of data even when much
less is required when I2C_FUNC_I2C is used. Reduce this to the correct
number.
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
drivers/usb/typec/tipd/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 21b3ae25c76d..c18ec3785592 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -123,7 +123,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
if (!tps->i2c_protocol)
return regmap_raw_read(tps->regmap, reg, val, len);
- ret = regmap_raw_read(tps->regmap, reg, data, sizeof(data));
+ ret = regmap_raw_read(tps->regmap, reg, data, len + 1);
if (ret)
return ret;
@@ -145,7 +145,7 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
data[0] = len;
memcpy(&data[1], val, len);
- return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+ return regmap_raw_write(tps->regmap, reg, data, len + 1);
}
static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] usb: typec: tipd: Add an additional overflow check
2021-09-14 10:42 [PATCH 1/2] usb: typec: tipd: Don't read/write more bytes than required Sven Peter
@ 2021-09-14 10:42 ` Sven Peter
2021-09-14 11:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Sven Peter @ 2021-09-14 10:42 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Sven Peter
tps6598x_block_read already checks for the maximum length of the read
but tps6598x_block_write does not. Add the symmetric check there as
well.
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
drivers/usb/typec/tipd/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index c18ec3785592..70e2d0d410c9 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
{
u8 data[TPS_MAX_LEN + 1];
+ if (WARN_ON(len + 1 > sizeof(data)))
+ return -EINVAL;
+
if (!tps->i2c_protocol)
return regmap_raw_write(tps->regmap, reg, val, len);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check
2021-09-14 10:42 ` [PATCH 2/2] usb: typec: tipd: Add an additional overflow check Sven Peter
@ 2021-09-14 11:00 ` Greg Kroah-Hartman
2021-09-14 11:18 ` Sven Peter
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-14 11:00 UTC (permalink / raw)
To: Sven Peter; +Cc: Heikki Krogerus, linux-usb, linux-kernel
On Tue, Sep 14, 2021 at 12:42:53PM +0200, Sven Peter wrote:
> tps6598x_block_read already checks for the maximum length of the read
> but tps6598x_block_write does not. Add the symmetric check there as
> well.
>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
> drivers/usb/typec/tipd/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index c18ec3785592..70e2d0d410c9 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> {
> u8 data[TPS_MAX_LEN + 1];
>
> + if (WARN_ON(len + 1 > sizeof(data)))
> + return -EINVAL;
No need to crash anything. If this is a valid thing for us to check,
let's check it and handle the error, but we should not reboot systems
that are running with panic-on-warn enabled, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check
2021-09-14 11:00 ` Greg Kroah-Hartman
@ 2021-09-14 11:18 ` Sven Peter
2021-09-14 11:23 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Sven Peter @ 2021-09-14 11:18 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Heikki Krogerus, linux-usb, linux-kernel
On Tue, Sep 14, 2021, at 13:00, Greg Kroah-Hartman wrote:
> On Tue, Sep 14, 2021 at 12:42:53PM +0200, Sven Peter wrote:
> > tps6598x_block_read already checks for the maximum length of the read
> > but tps6598x_block_write does not. Add the symmetric check there as
> > well.
> >
> > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> > ---
> > drivers/usb/typec/tipd/core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> > index c18ec3785592..70e2d0d410c9 100644
> > --- a/drivers/usb/typec/tipd/core.c
> > +++ b/drivers/usb/typec/tipd/core.c
> > @@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> > {
> > u8 data[TPS_MAX_LEN + 1];
> >
> > + if (WARN_ON(len + 1 > sizeof(data)))
> > + return -EINVAL;
>
> No need to crash anything. If this is a valid thing for us to check,
> let's check it and handle the error, but we should not reboot systems
> that are running with panic-on-warn enabled, right?
Sure, that makes sense. I guess the same point applies to the WARN_ON in
the same check in tps6598x_block_read. I can add a patch to remove that one
to v2 as well.
thanks,
Sven
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check
2021-09-14 11:18 ` Sven Peter
@ 2021-09-14 11:23 ` Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-14 11:23 UTC (permalink / raw)
To: Sven Peter; +Cc: Heikki Krogerus, linux-usb, linux-kernel
On Tue, Sep 14, 2021 at 01:18:16PM +0200, Sven Peter wrote:
>
>
> On Tue, Sep 14, 2021, at 13:00, Greg Kroah-Hartman wrote:
> > On Tue, Sep 14, 2021 at 12:42:53PM +0200, Sven Peter wrote:
> > > tps6598x_block_read already checks for the maximum length of the read
> > > but tps6598x_block_write does not. Add the symmetric check there as
> > > well.
> > >
> > > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> > > ---
> > > drivers/usb/typec/tipd/core.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> > > index c18ec3785592..70e2d0d410c9 100644
> > > --- a/drivers/usb/typec/tipd/core.c
> > > +++ b/drivers/usb/typec/tipd/core.c
> > > @@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
> > > {
> > > u8 data[TPS_MAX_LEN + 1];
> > >
> > > + if (WARN_ON(len + 1 > sizeof(data)))
> > > + return -EINVAL;
> >
> > No need to crash anything. If this is a valid thing for us to check,
> > let's check it and handle the error, but we should not reboot systems
> > that are running with panic-on-warn enabled, right?
>
> Sure, that makes sense. I guess the same point applies to the WARN_ON in
> the same check in tps6598x_block_read. I can add a patch to remove that one
> to v2 as well.
That would be great, thanks.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-14 11:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 10:42 [PATCH 1/2] usb: typec: tipd: Don't read/write more bytes than required Sven Peter
2021-09-14 10:42 ` [PATCH 2/2] usb: typec: tipd: Add an additional overflow check Sven Peter
2021-09-14 11:00 ` Greg Kroah-Hartman
2021-09-14 11:18 ` Sven Peter
2021-09-14 11:23 ` Greg Kroah-Hartman
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.