Skip to contentSkip to conversation

rpc: fix garbage in printk in svc_tcp_accept()

PATCH In review J. Bruce Fields 2 participants 3 messages Sep 20, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rpc: fix garbage in printk in svc_tcp_accept()

we upgraded the kernel of a nfs-server from 2.6.17.11 to 2.6.22.6. Since then we get the message

lockd: too many open TCP sockets, consider increasing the number of nfsd threads
lockd: last TCP connect from ^\\236^\É^D

These random characters in the second line are caused by a bug in svc_tcp_accept.

(Note: there are two previous __svc_print_addr(sin, buf, sizeof(buf)) calls in this function, either of which would initialize buf correctly; but both are inside "if"'s and are not necessarily executed. This is less obvious in the second case, which is inside a dprintk(), which is a macro which expands to an if statement.)

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

We're printk'ing random uninitialized memory here, which I assume makes it appropriate for both 2.6.23 and 2.6.22.x.--b.

net/sunrpc/svcsock.c +2 -1
index 1a89992..036ab52 100644
@@ -1110,7 +1110,8 @@ svc_tcp_accept(struct svc_sock *svsk)
 						   serv->sv_name);
 				printk(KERN_NOTICE
 				       "%s: last TCP connect from %s\n",
-				       serv->sv_name, buf);
+				       serv->sv_name, __svc_print_addr(sin,
+							buf, sizeof(buf)));
 			}
 			/*
 			 * Always select the oldest socket. It's not fair,
Signature · 1 line
1.5.3.1.139.g9346b
#1 lore raw
Quoted 3 lines · J. Bruce Fields

we upgraded the kernel of a nfs-server from 2.6.17.11 to 2.6.22.6. Since then we get the message

...

Bruce - you're losing authorship information.

Please don't do that.

Put a

From: Wolfgang Walter <[email protected]>

at the top of the email, so that the author gets properly attributed. I don't know how many of these I've missed - in this case I just ended up googling for the commit message because the "Signed-off-by:" lines implied that you weren't the original author, and see the real patch authorship that way.

But it's *not* the case that the first sign-off is always the author, and I don't want to need to google for each patch I get, so please make it a habit to make sure that there's proper attribution for the patches.

Some people (read: Andrew) go so far as to *always* do it (ie Andrew puts a "From: Andrew Morton .." at the heads of the emails he sends out), and I appreciate it. That way, even if the email then gets forwarded by somebody else (because Andrew sent it to some submaintainer instead of me), the original authorship doesn't get lost when somebody else then forwards the email.

So that "always explicitly say who the author is" is a good idea, but when forwarding somebody elses patch, it's *more* than a good idea - at that point it's absolutely required for proper attribution.

Linus

#2 lore raw
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · J. Bruce Fields

we upgraded the kernel of a nfs-server from 2.6.17.11 to 2.6.22.6. Since then we get the message

...

Bruce - you're losing authorship information.

Whoops, yes, I usually rely on git-send-email to do that, but was sending the patch by hand this time in a hurry, reading the commit into my mail client, and forgot the From: line. Thanks for catching that.

Quoted 3 lines

So that "always explicitly say who the author is" is a good idea, but when forwarding somebody elses patch, it's *more* than a good idea - at that point it's absolutely required for proper attribution.

Yeah, I know. Apologies.--b.