Re: splat in ikheaders_read (bpftrace)

[Jakub Kicinski <kuba@xxxxxxxxxx>]

On Thu, 02 Mar 2023 13:57:57 -0800 Kees Cook wrote:
> On March 2, 2023 11:21:30 AM PST, Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> >Hi Kees!
> >
> >Running tests on net (Linus's tree as of Monday) I get this splat
> >trying to attach bpftrace to a tracepoint:  
> 
> Can you give me an example command line to try to repro this?

cat /sys/kernel/kheaders.tar.xz >> /dev/null

> >[ 2468.945793] kernel BUG at lib/string_helpers.c:1027!  
> 
> Were there any lines above this? It must be memcpy blowing up due to what it thinks is an overflow from having tracked an allocation size (that's the major change this cycle).

Yes

  detected buffer overflow in memcpy

sorry.

> >[ 2468.949683] RIP: 0010:fortify_panic+0xf/0x20
> >...
> >[ 2468.961930] Call Trace:
> >[ 2468.962300]  <TASK>
> >[ 2468.962611]  ikheaders_read+0x45/0x50 [kheaders]  
> 
> static ssize_t
> ikheaders_read(struct file *file,  struct kobject *kobj,
> 	       struct bin_attribute *bin_attr,
> 	       char *buf, loff_t off, size_t len)
> {
> 	memcpy(buf, &kernel_headers_data + off, len);
> 	return len;
> }
> 
> I will take a look at the caller's allocation of "buf" and kernel_headers_data.

Mm. Actually stopping to look at the code - I don't see it bound
checking against kernel_headers_data_end :| Maybe we need:

@@ -34,6 +35,7 @@ ikheaders_read(struct file *file,  struct kobject *kobj,
               struct bin_attribute *bin_attr,
               char *buf, loff_t off, size_t len)
 {
+       len = min_t(size_t, kernel_headers_data_end - kernel_headers_data, len);
        memcpy(buf, &kernel_headers_data + off, len);
        return len;
 }