Re: [PATCH 04/24] SUNRPC: Address potential buffer length overflow in svc_sendto

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Apr 14, 2008, at 1:48 PM, J. Bruce Fields wrote:
On Mon, Apr 14, 2008 at 12:27:08PM -0400, Chuck Lever wrote:
Paranoia: Ensure a negative error value return from kernel_sendpage never
matches a large buffer length.

That is a little paranoid. Absent an argument for exactly what sort of bug could allow us to reach this point with the head iov_len in at least
the gigabytes, I'm inclined to leave this alone for simplicity's
sake....

I would like to document that we have noticed the mixed sign comparison there, and that it has been made entirely safe. In certain cases, the code as it stands does not do what it looks like it does.

I'm not recommending these changes to be pedantic, nor am I doing this for my own health. When we leave checks like this out, what we have is "clever" nondeterministic code rather than safe code. This is simply good software engineering, but every one of these I've proposed has been argued over and often rejected.

We know that humans write this code, and that humans make mistakes. We should be doing everything we can to make this code less vulnerable to coding mistakes. This is not about _fixing_ bugs, it's about _preventing_ them.

Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---

net/sunrpc/svcsock.c |    2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 6d4162b..a8ae279 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -200,7 +200,7 @@ static int svc_sendto(struct svc_rqst *rqstp, struct xdr_buf *xdr)
		flags = 0;
	len = kernel_sendpage(sock, rqstp->rq_respages[0], 0,
				  xdr->head[0].iov_len, flags);
-	if (len != xdr->head[0].iov_len)
+	if (len < 0 || len != xdr->head[0].iov_len)
		goto out;
	slen -= xdr->head[0].iov_len;
	if (slen == 0)


--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux