[PATCH] don't expect inode name to be NUL-terminated (avoid read overrun)
Jim Meyering <[email protected]> Fri, 10 Sep 2010 14:55:54 +0200
| Newsgroups | org.kernel.vger.hail-devel |
|---|---|
| Message-ID | <[email protected]> |
* server/msg.c (msg_get): Copy only name_len bytes, then NUL-terminate,
rather than using snprintf to copy up to and including nonexistent NUL.
---
valgrind exposed this. The use of snprintf would have been
correct if the inode name buffer (following the struct raw_inode)
were NUL-terminated, but it is not.
Invalid read of size 1
at 0x3502647FF7: vfprintf (vfprintf.c:1593)
by 0x350266EFB1: vsnprintf (vsnprintf.c:120)
by 0x350264F022: snprintf (snprintf.c:35)
by 0x4061D5: msg_get (msg.c:451)
by 0x407FD9: udp_rx_handle (server.c:164)
by 0x408244: udp_rx (server.c:233)
by 0x4091AA: udp_srv_event (server.c:640)
by 0x409DE6: main_loop (server.c:1026)
by 0x40A17E: main (server.c:1135)
Address 0x4d2afae is 0 bytes after a block of size 62 alloc'd
at 0x4A0515D: malloc (vg_replace_malloc.c:195)
by 0x3505F35527: __os_umalloc (os_alloc.c:68)
by 0x3505EF8F8D: __db_retcopy (db_ret.c:124)
by 0x3505EF90EB: __db_ret (db_ret.c:69)
by 0x3505ED93A7: __dbc_iget (db_cam.c:1111)
by 0x3505EE5CB3: __db_get (db_iface.c:779)
by 0x3505EE5FDA: __db_get_pp (db_iface.c:694)
by 0x4040F6: cldb_inode_get (cldb.c:537)
by 0x406069: msg_get (msg.c:430)
by 0x407FD9: udp_rx_handle (server.c:164)
by 0x408244: udp_rx (server.c:233)
by 0x4091AA: udp_srv_event (server.c:640)
by 0x409DE6: main_loop (server.c:1026)
by 0x40A17E: main (server.c:1135)
Here's a fix:
server/msg.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/msg.c b/server/msg.c
index f2dda59..8abb1e6 100644
--- a/server/msg.c
+++ b/server/msg.c
@@ -448,7 +448,8 @@ void msg_get(struct session *sess, const void *v)
name_len = le32_to_cpu(inode->ino_len);
inode_name = alloca(name_len + 1);
- snprintf(inode_name, name_len + 1, "%s", (char *)(inode + 1));
+ memcpy (inode_name, inode + 1, name_len);
+ inode_name[name_len] = 0;
resp.inode_name = inode_name;
resp.data.data_len = 0;
@@ -1172,4 +1173,3 @@ err_out_noabort:
sess_sendresp_generic(sess, resp_rc);
free(h);
}
-
--
1.7.3.rc0.183.gb0497