linux-kernel - Re: [GIT PULL] ftrace: Check if pages were allocated before calling free

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wg_6JBGSiKtQp7CT99kFt2L5b1sbvs6YBTXJAWdPc8Hig@mail.gmail.com>
Date:   Thu, 1 Apr 2021 13:18:59 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] ftrace: Check if pages were allocated before calling free_pages()

On Thu, Apr 1, 2021 at 1:07 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 31 Mar 2021 11:03:21 -0700
> Linus Torvalds <torvalds@...ux-foundation.org> wrote:
>
> > @@ -6231,7 +6231,8 @@ static int ftrace_process_locs(struct module *mod,
> >               if (!addr)
> >                       continue;
> >
> > -             if (pg->index == pg->size) {
> > +             end_offset = (pg->index+1) * sizeof(pg->records[0]);
> > +             if (end_offset < PAGE_SIZE << pg->order) {
>
> I believe that needs to be:
>
>         if (end_offset >= PAGE_SIZE << pg->order) {

No, but the "<" should be ">". That was just a typo.

It's ok for end_offset to be at the edge. That's the "we filled the
pages completely".

I'm not sure that can actually happen (it depends on the size of the
structure, and whether the size of the allocation is divisible by it),
but it's not wrong if it does.

Think of it this way: imagine that we have one 4kB page, and the size
of the structure is 1kB in size. You can fit 4 structures in it, and
end_offset for the last one will be index=3, so that you'll have:

        end_offset = (pg->index+1) * sizeof(pg->records[0]);

which will be

        end_offset = (3+1) * 1024;

ie 4096. That just means that the struct fill fill things _up_to_ the
end of the page.

So only when the end_offset is strictly larger than the page would it
have overflowed the allocation.

             Linus