Messages in this thread | | | Date | Wed, 29 Nov 2017 14:36:37 +0900 | From | Joonsoo Kim <> | Subject | Re: [PATCH 02/18] vchecker: introduce the valid access checker |
| |
On Tue, Nov 28, 2017 at 11:41:08AM -0800, Andi Kleen wrote: > js1304@gmail.com writes: > > > From: Joonsoo Kim <iamjoonsoo.kim@lge.com> > > Looks useful. Essentially unlimited hardware break points, combined > with slab.
Thanks!!!
> > Didn't do a full review, but noticed some things below. > > + > > + buf = kmalloc(PAGE_SIZE, GFP_KERNEL); > > + if (!buf) > > + return -ENOMEM; > > + > > + if (copy_from_user(buf, ubuf, cnt)) { > > + kfree(buf); > > + return -EFAULT; > > + } > > + > > + if (isspace(buf[0])) > > + remove = true; > > and that may be uninitialized.
I will add 'cnt == 0' check above.
> and the space changes the operation? That's a strange syntax.
Intention is to clear the all the previous configuration when user input is '\n'. Will fix it by checking '\n' directly.
> > > + buf[cnt - 1] = '\0'; > > That's an underflow of one byte if cnt is 0.
Will add 'cnt == 0' check above.
String parsing part in this patchset will not work properly when the last input character is not '\n'. I will fix it on the next spin.
Thanks.
| |