Skip to content

Commit 19e506b

Browse files
geertugregkh
authored andcommitted
eeprom: at25: Rework buggy read splitting
The recent change to split reads into chunks has several problems: 1. If an SPI controller has no transfer size limit, max_chunk is SIZE_MAX, and num_msgs becomes zero, causing no data to be read into the buffer, and exposing the original contents of the buffer to userspace, 2. If the requested read size is not a multiple of the maximum transfer size, the last transfer reads too much data, overflowing the buffer, 3. The loop logic differs from the write case. Fix the above by: 1. Keeping track of the number of bytes that are still to be transferred, instead of precalculating the number of messages and keeping track of the number of bytes tranfered, 2. Calculating the transfer size of each individual message, taking into account the number of bytes left, 3. Switching from a "while"-loop to a "do-while"-loop, and renaming "msg_count" to "segment". While at it, drop the superfluous cast from "unsigned int" to "unsigned int", also from at25_ee_write(), where it was probably copied from. Fixes: 0a35780 ("eeprom: at25: Split reads into chunks and cap write size") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Link: https://lore.kernel.org/r/7ae260778d2c08986348ea48ce02ef148100e088.1655817534.git.geert+renesas@glider.be Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 03c765b commit 19e506b

File tree

1 file changed

+12
-14
lines changed

1 file changed

+12
-14
lines changed

drivers/misc/eeprom/at25.c

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,9 @@ static int at25_ee_read(void *priv, unsigned int offset,
8080
struct at25_data *at25 = priv;
8181
char *buf = val;
8282
size_t max_chunk = spi_max_transfer_size(at25->spi);
83-
size_t num_msgs = DIV_ROUND_UP(count, max_chunk);
84-
size_t nr_bytes = 0;
85-
unsigned int msg_offset;
86-
size_t msg_count;
83+
unsigned int msg_offset = offset;
84+
size_t bytes_left = count;
85+
size_t segment;
8786
u8 *cp;
8887
ssize_t status;
8988
struct spi_transfer t[2];
@@ -97,9 +96,8 @@ static int at25_ee_read(void *priv, unsigned int offset,
9796
if (unlikely(!count))
9897
return -EINVAL;
9998

100-
msg_offset = (unsigned int)offset;
101-
msg_count = min(count, max_chunk);
102-
while (num_msgs) {
99+
do {
100+
segment = min(bytes_left, max_chunk);
103101
cp = at25->command;
104102

105103
instr = AT25_READ;
@@ -131,8 +129,8 @@ static int at25_ee_read(void *priv, unsigned int offset,
131129
t[0].len = at25->addrlen + 1;
132130
spi_message_add_tail(&t[0], &m);
133131

134-
t[1].rx_buf = buf + nr_bytes;
135-
t[1].len = msg_count;
132+
t[1].rx_buf = buf;
133+
t[1].len = segment;
136134
spi_message_add_tail(&t[1], &m);
137135

138136
status = spi_sync(at25->spi, &m);
@@ -142,10 +140,10 @@ static int at25_ee_read(void *priv, unsigned int offset,
142140
if (status)
143141
return status;
144142

145-
--num_msgs;
146-
msg_offset += msg_count;
147-
nr_bytes += msg_count;
148-
}
143+
msg_offset += segment;
144+
buf += segment;
145+
bytes_left -= segment;
146+
} while (bytes_left > 0);
149147

150148
dev_dbg(&at25->spi->dev, "read %zu bytes at %d\n",
151149
count, offset);
@@ -229,7 +227,7 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
229227
do {
230228
unsigned long timeout, retries;
231229
unsigned segment;
232-
unsigned offset = (unsigned) off;
230+
unsigned offset = off;
233231
u8 *cp = bounce;
234232
int sr;
235233
u8 instr;

0 commit comments

Comments
 (0)