Skip to content

Commit d0259a8

Browse files
committed
9p/net: fix improper handling of bogus negative read/write replies
In p9_client_write() and p9_client_read_once(), if the server incorrectly replies with success but a negative write/read count then we would consider written (negative) <= rsize (positive) because both variables were signed. Make variables unsigned to avoid this problem. The reproducer linked below now fails with the following error instead of a null pointer deref: 9pnet: bogus RWRITE count (4294967295 > 3) Reported-by: Robert Morris <rtm@mit.edu> Closes: https://lore.kernel.org/16271.1734448631@26-5-164.dynamic.csail.mit.edu Message-ID: <20250319-9p_unsigned_rw-v3-1-71327f1503d0@codewreck.org> Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
1 parent 3f61ac7 commit d0259a8

File tree

1 file changed

+16
-14
lines changed

1 file changed

+16
-14
lines changed

net/9p/client.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,7 +1548,8 @@ p9_client_read_once(struct p9_fid *fid, u64 offset, struct iov_iter *to,
15481548
struct p9_client *clnt = fid->clnt;
15491549
struct p9_req_t *req;
15501550
int count = iov_iter_count(to);
1551-
int rsize, received, non_zc = 0;
1551+
u32 rsize, received;
1552+
bool non_zc = false;
15521553
char *dataptr;
15531554

15541555
*err = 0;
@@ -1571,7 +1572,7 @@ p9_client_read_once(struct p9_fid *fid, u64 offset, struct iov_iter *to,
15711572
0, 11, "dqd", fid->fid,
15721573
offset, rsize);
15731574
} else {
1574-
non_zc = 1;
1575+
non_zc = true;
15751576
req = p9_client_rpc(clnt, P9_TREAD, "dqd", fid->fid, offset,
15761577
rsize);
15771578
}
@@ -1592,11 +1593,11 @@ p9_client_read_once(struct p9_fid *fid, u64 offset, struct iov_iter *to,
15921593
return 0;
15931594
}
15941595
if (rsize < received) {
1595-
pr_err("bogus RREAD count (%d > %d)\n", received, rsize);
1596+
pr_err("bogus RREAD count (%u > %u)\n", received, rsize);
15961597
received = rsize;
15971598
}
15981599

1599-
p9_debug(P9_DEBUG_9P, "<<< RREAD count %d\n", received);
1600+
p9_debug(P9_DEBUG_9P, "<<< RREAD count %u\n", received);
16001601

16011602
if (non_zc) {
16021603
int n = copy_to_iter(dataptr, received, to);
@@ -1623,17 +1624,17 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
16231624
*err = 0;
16241625

16251626
while (iov_iter_count(from)) {
1626-
int count = iov_iter_count(from);
1627-
int rsize = fid->iounit;
1628-
int written;
1627+
size_t count = iov_iter_count(from);
1628+
u32 rsize = fid->iounit;
1629+
u32 written;
16291630

16301631
if (!rsize || rsize > clnt->msize - P9_IOHDRSZ)
16311632
rsize = clnt->msize - P9_IOHDRSZ;
16321633

16331634
if (count < rsize)
16341635
rsize = count;
16351636

1636-
p9_debug(P9_DEBUG_9P, ">>> TWRITE fid %d offset %llu count %d (/%d)\n",
1637+
p9_debug(P9_DEBUG_9P, ">>> TWRITE fid %d offset %llu count %u (/%zu)\n",
16371638
fid->fid, offset, rsize, count);
16381639

16391640
/* Don't bother zerocopy for small IO (< 1024) */
@@ -1659,11 +1660,11 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
16591660
break;
16601661
}
16611662
if (rsize < written) {
1662-
pr_err("bogus RWRITE count (%d > %d)\n", written, rsize);
1663+
pr_err("bogus RWRITE count (%u > %u)\n", written, rsize);
16631664
written = rsize;
16641665
}
16651666

1666-
p9_debug(P9_DEBUG_9P, "<<< RWRITE count %d\n", written);
1667+
p9_debug(P9_DEBUG_9P, "<<< RWRITE count %u\n", written);
16671668

16681669
p9_req_put(clnt, req);
16691670
iov_iter_revert(from, count - written - iov_iter_count(from));
@@ -2098,7 +2099,8 @@ EXPORT_SYMBOL_GPL(p9_client_xattrcreate);
20982099

20992100
int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset)
21002101
{
2101-
int err, rsize, non_zc = 0;
2102+
int err, non_zc = 0;
2103+
u32 rsize;
21022104
struct p9_client *clnt;
21032105
struct p9_req_t *req;
21042106
char *dataptr;
@@ -2107,7 +2109,7 @@ int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset)
21072109

21082110
iov_iter_kvec(&to, ITER_DEST, &kv, 1, count);
21092111

2110-
p9_debug(P9_DEBUG_9P, ">>> TREADDIR fid %d offset %llu count %d\n",
2112+
p9_debug(P9_DEBUG_9P, ">>> TREADDIR fid %d offset %llu count %u\n",
21112113
fid->fid, offset, count);
21122114

21132115
clnt = fid->clnt;
@@ -2142,11 +2144,11 @@ int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset)
21422144
goto free_and_error;
21432145
}
21442146
if (rsize < count) {
2145-
pr_err("bogus RREADDIR count (%d > %d)\n", count, rsize);
2147+
pr_err("bogus RREADDIR count (%u > %u)\n", count, rsize);
21462148
count = rsize;
21472149
}
21482150

2149-
p9_debug(P9_DEBUG_9P, "<<< RREADDIR count %d\n", count);
2151+
p9_debug(P9_DEBUG_9P, "<<< RREADDIR count %u\n", count);
21502152

21512153
if (non_zc)
21522154
memmove(data, dataptr, count);

0 commit comments

Comments
 (0)