From: Jim Meyering <jim@meyering.net>
To: hail-devel@vger.kernel.org
Subject: [PATCH] don't expect inode name to be NUL-terminated (avoid read overrun)
Date: Fri, 10 Sep 2010 14:55:54 +0200 [thread overview]
Message-ID: <874odxep0l.fsf@meyering.net> (raw)
* 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
next reply other threads:[~2010-09-10 12:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-10 12:55 Jim Meyering [this message]
2010-09-10 17:01 ` [PATCH] don't expect inode name to be NUL-terminated (avoid read overrun) Colin McCabe
2010-09-14 17:44 ` Jeff Garzik
2010-09-15 8:15 ` Jim Meyering
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874odxep0l.fsf@meyering.net \
--to=jim@meyering.net \
--cc=hail-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).