Linux I2C development
 help / color / mirror / Atom feed
* [PATCH] w1: ds28e17: reject an oversize length on an I2C block read
@ 2026-06-29 12:10 Maoyi Xie
  2026-07-02 16:38 ` Andi Shyti
  0 siblings, 1 reply; 4+ messages in thread
From: Maoyi Xie @ 2026-06-29 12:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Jan Kandziora
  Cc: Wolfram Sang, Andi Shyti, Bjorn Andersson, linux-i2c,
	linux-kernel

w1_f19_i2c_master_transfer() is the master_xfer for the DS28E17 1-Wire
to I2C bridge. On an I2C_M_RECV_LEN read, it takes the length from the
device. The downstream slave puts a length byte in buf[0]. The driver
then reads that many bytes into buf[1] with w1_f19_i2c_read().

buf[0] is controlled by the device and can be 0 to 255.
w1_f19_i2c_read() only rejects a zero count. The caller buffer is
I2C_SMBUS_BLOCK_MAX + 2, so 34 bytes. A length above 32 makes the read
run past it, up to about 222 bytes out of bounds.

The SMBus core does check buf[0] against I2C_SMBUS_BLOCK_MAX. That
check runs after master_xfer returns. By then the write is already
done. i2c-algo-bit rejects an oversize length before it copies, and
returns -EPROTO.

Reject a length above I2C_SMBUS_BLOCK_MAX at both RECV_LEN sites, the
same way i2c-algo-bit does.

Fixes: ebc4768ac497 ("add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge")
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 drivers/w1/slaves/w1_ds28e17.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/w1/slaves/w1_ds28e17.c b/drivers/w1/slaves/w1_ds28e17.c
index e53bc41bde3ca..b638963d4b595 100644
--- a/drivers/w1/slaves/w1_ds28e17.c
+++ b/drivers/w1/slaves/w1_ds28e17.c
@@ -389,6 +389,10 @@ static int w1_f19_i2c_master_transfer(struct i2c_adapter *adapter,
 			 * another simple read in that case.
 			 */
 			if (msgs[i+1].flags & I2C_M_RECV_LEN) {
+				if (msgs[i+1].buf[0] > I2C_SMBUS_BLOCK_MAX) {
+					i = -EPROTO;
+					goto error;
+				}
 				result = w1_f19_i2c_read(sl, msgs[i+1].addr,
 					&(msgs[i+1].buf[1]), msgs[i+1].buf[0]);
 				if (result < 0) {
@@ -415,6 +419,10 @@ static int w1_f19_i2c_master_transfer(struct i2c_adapter *adapter,
 			 * another simple read in that case.
 			 */
 			if (msgs[i].flags & I2C_M_RECV_LEN) {
+				if (msgs[i].buf[0] > I2C_SMBUS_BLOCK_MAX) {
+					i = -EPROTO;
+					goto error;
+				}
 				result = w1_f19_i2c_read(sl,
 					msgs[i].addr,
 					&(msgs[i].buf[1]),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] w1: ds28e17: reject an oversize length on an I2C block read
  2026-06-29 12:10 [PATCH] w1: ds28e17: reject an oversize length on an I2C block read Maoyi Xie
@ 2026-07-02 16:38 ` Andi Shyti
  2026-07-02 16:41   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 4+ messages in thread
From: Andi Shyti @ 2026-07-02 16:38 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Krzysztof Kozlowski, Jan Kandziora, Wolfram Sang, Bjorn Andersson,
	linux-i2c, linux-kernel

Hi Maoyi,

On Mon, Jun 29, 2026 at 08:10:43PM +0800, Maoyi Xie wrote:
> w1_f19_i2c_master_transfer() is the master_xfer for the DS28E17 1-Wire
> to I2C bridge. On an I2C_M_RECV_LEN read, it takes the length from the
> device. The downstream slave puts a length byte in buf[0]. The driver
> then reads that many bytes into buf[1] with w1_f19_i2c_read().

yes, I2C_M_RECV_LEN is tied to the SMBUS so that it needs to be
capped to 32 bytes.

> buf[0] is controlled by the device and can be 0 to 255.
> w1_f19_i2c_read() only rejects a zero count. The caller buffer is
> I2C_SMBUS_BLOCK_MAX + 2, so 34 bytes. A length above 32 makes the read
> run past it, up to about 222 bytes out of bounds.
> 
> The SMBus core does check buf[0] against I2C_SMBUS_BLOCK_MAX. That
> check runs after master_xfer returns. By then the write is already
> done. i2c-algo-bit rejects an oversize length before it copies, and
> returns -EPROTO.
> 
> Reject a length above I2C_SMBUS_BLOCK_MAX at both RECV_LEN sites, the
> same way i2c-algo-bit does.
> 
> Fixes: ebc4768ac497 ("add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge")
> Cc: stable@vger.kernel.org

Cc: <stable@vger.kernel.org> # v4.15+

> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

This should go through Krzysztof, I guess.

Thanks,
Andi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] w1: ds28e17: reject an oversize length on an I2C block read
  2026-07-02 16:38 ` Andi Shyti
@ 2026-07-02 16:41   ` Krzysztof Kozlowski
  2026-07-02 19:07     ` Andi Shyti
  0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-02 16:41 UTC (permalink / raw)
  To: Andi Shyti, Maoyi Xie
  Cc: Jan Kandziora, Wolfram Sang, Bjorn Andersson, linux-i2c,
	linux-kernel

On 02/07/2026 18:38, Andi Shyti wrote:
>>
>> Reject a length above I2C_SMBUS_BLOCK_MAX at both RECV_LEN sites, the
>> same way i2c-algo-bit does.
>>
>> Fixes: ebc4768ac497 ("add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge")
>> Cc: stable@vger.kernel.org
> 
> Cc: <stable@vger.kernel.org> # v4.15+

This is not needed. CC stable is enough with Fixes tag. Unless this
should not be backported to the kernel which introduced this issue, but
then why?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] w1: ds28e17: reject an oversize length on an I2C block read
  2026-07-02 16:41   ` Krzysztof Kozlowski
@ 2026-07-02 19:07     ` Andi Shyti
  0 siblings, 0 replies; 4+ messages in thread
From: Andi Shyti @ 2026-07-02 19:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Maoyi Xie, Jan Kandziora, Wolfram Sang, Bjorn Andersson,
	linux-i2c, linux-kernel

Hi Krzysztof,

> >> Reject a length above I2C_SMBUS_BLOCK_MAX at both RECV_LEN sites, the
> >> same way i2c-algo-bit does.
> >>
> >> Fixes: ebc4768ac497 ("add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge")
> >> Cc: stable@vger.kernel.org
> > 
> > Cc: <stable@vger.kernel.org> # v4.15+
> 
> This is not needed. CC stable is enough with Fixes tag. Unless this
> should not be backported to the kernel which introduced this issue, but
> then why?

yes, you're right, but reading the documentation it suggests to
use this form, it's easy to get, so why not? I always use it.

Andi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-02 19:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 12:10 [PATCH] w1: ds28e17: reject an oversize length on an I2C block read Maoyi Xie
2026-07-02 16:38 ` Andi Shyti
2026-07-02 16:41   ` Krzysztof Kozlowski
2026-07-02 19:07     ` Andi Shyti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox