linux-kernel - Re: [PATCH] tracing: fix oob write in trace_seq_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAO9qdTE4gXLorLXLovem1r7uVFSJ4FjpNHM9tH2C2geruQqG9Q@mail.gmail.com>
Date: Mon, 21 Apr 2025 23:36:52 +0900
From: Jeongjun Park <aha310510@...il.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: mhiramat@...nel.org, linux-kernel@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org, 
	syzbot+c8cd2d2c412b868263fb@...kaller.appspotmail.com
Subject: Re: [PATCH] tracing: fix oob write in trace_seq_to_buffer()

Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Mon, 21 Apr 2025 22:49:36 +0900
> Jeongjun Park <aha310510@...il.com> wrote:
>
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 8ddf6b17215c..8ba6ea38411d 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1827,6 +1827,8 @@ static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt)
> >       len = trace_seq_used(s) - s->readpos;
> >       if (cnt > len)
> >               cnt = len;
> > +     if (cnt > PAGE_SIZE)
> > +             return -EINVAL;
>
> You fixed the wrong location. The caller should know how much size the
> buffer is, and that's passed in by cnt.
>
> >       memcpy(buf, s->buffer + s->readpos, cnt);
> >
> >       s->readpos += cnt;
>
> The correct fix would be:
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b6e40e8791fa..c23b5ab27314 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6729,7 +6864,8 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
>                 /* Copy the data into the page, so we can start over. */
>                 ret = trace_seq_to_buffer(&iter->seq,
>                                           page_address(spd.pages[i]),
> -                                         trace_seq_used(&iter->seq));
> +                                         min(trace_seq_used(&iter->seq),
> +                                             PAGE_SIZE));
>                 if (ret < 0) {
>                         __free_page(spd.pages[i]);
>                         break;
>
> Especially since the trace_seq_to_buffer() code should be moved out of this
> file and should have no idea how big the buffer passed in is.

Thanks for your advice! I'll fix it right away and send you a v2 patch.

Regards,

Jeongjun Park

>
> -- Steve