Messages in this thread | | | Date | Fri, 3 Apr 2009 14:52:50 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] blktrace: fix a bug in blk_msg_write() |
| |
* Jens Axboe <jens.axboe@oracle.com> wrote:
> On Fri, Apr 03 2009, Ingo Molnar wrote: > > > > * Li Zefan <lizf@cn.fujitsu.com> wrote: > > > > > This is another long-standing bug. > > > > > > ================ > > > > > > > > > (console 1) > > > # echo -n 'a' > /sys/kernel/debug/block/sda/msg > > > (console 2) > > > # blktrace -d /dev/sda -a pc -o - | blkparse -i - > > > 8,0 0 0 0.000000000 0 m N a??????????????????@?????? > > > > > > We should terminate the msg buffer with '\0'. > > > > > > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> > > > --- > > > kernel/trace/blktrace.c | 5 +++-- > > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > > index c0ef70d..2bf341f 100644 > > > --- a/kernel/trace/blktrace.c > > > +++ b/kernel/trace/blktrace.c > > > @@ -327,10 +327,10 @@ static ssize_t blk_msg_write(struct file *filp, const char __user *buffer, > > > char *msg; > > > struct blk_trace *bt; > > > > > > - if (count > BLK_TN_MAX_MSG) > > > + if (count >= BLK_TN_MAX_MSG) > > > return -EINVAL; > > > > > > - msg = kmalloc(count, GFP_KERNEL); > > > + msg = kmalloc(count + 1, GFP_KERNEL); > > > if (msg == NULL) > > > return -ENOMEM; > > > > > > @@ -338,6 +338,7 @@ static ssize_t blk_msg_write(struct file *filp, const char __user *buffer, > > > kfree(msg); > > > return -EFAULT; > > > } > > > + msg[count] = '\0'; > > > > > > bt = filp->private_data; > > > __trace_note_message(bt, "%s", msg); > > > > Nice fix! > > > > Jens, do you ack this and should i add a Cc: <stable@kernel.org> > > backporting tag? > > > > This fix applies fine to v2.6.29 after: > > > > sed 's/kernel\/trace\/blktrace.c/block\/blktrace.c' > > This is identical to the following patch from Carl Henrik Lunde, which > he just ported to -tip. It has existed before, as a patch to 2.6.29-git, > but was unfortunately dropped. > > So is this a coincidence? If not, Carl Henrik Lunde should be credited. > > >From f116bce73e04d01614fd28ea678e0953cc321155 Mon Sep 17 00:00:00 2001 > From: Carl Henrik Lunde <chlunde@ping.uio.no> > Date: Wed, 25 Mar 2009 14:23:16 +0100 > Subject: [PATCH] blktrace: NUL-terminate user space messages > > Make sure messages from user space are NUL-terminated strings, > otherwise we could dump random memory to the block trace file. > Additionally, I've limited the message to BLK_TN_MAX_MSG-1 characters, > because the last character would be stripped by vscnprintf anyway. > > Signed-off-by: Carl Henrik Lunde <chlunde@ping.uio.no>
Sure, i've picked up Carl's version of the fix instead, it dates back to early February!
( Li Zefan's fix was a tiny notch cleaner - i picked that cleanup up on top of Carl's patch separately. )
Thanks,
Ingo
| |