linux-kernel - Re: [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to 12 for TRACING

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]

Date:   Thu, 22 Jun 2023 21:05:55 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     Yonghong Song <yhs@...a.com>,
        "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "martin.lau@...ux.dev" <martin.lau@...ux.dev>,
        "song@...nel.org" <song@...nel.org>, "yhs@...com" <yhs@...com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "sdf@...gle.com" <sdf@...gle.com>,
        "haoluo@...gle.com" <haoluo@...gle.com>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "benbjiang@...cent.com" <benbjiang@...cent.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Menglong Dong <imagedong@...cent.com>
Subject: Re: [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to
 12 for TRACING

On Thu, Jun 22, 2023 at 5:06 PM David Laight <David.Laight@...lab.com> wrote:
>
> ...
> > > +   /* Generally speaking, the compiler will pass the arguments
> > > +    * on-stack with "push" instruction, which will take 8-byte
> > > +    * on the stack. On this case, there won't be garbage values
> >
> > On this case -> In this case. The same for below another case.
> >
> > > +    * while we copy the arguments from origin stack frame to current
> > > +    * in BPF_DW.
> > > +    *
> > > +    * However, sometimes the compiler will only allocate 4-byte on
> > > +    * the stack for the arguments. For now, this case will only
> > > +    * happen if there is only one argument on-stack and its size
> > > +    * not more than 4 byte. On this case, there will be garbage
> > > +    * values on the upper 4-byte where we store the argument on
> > > +    * current stack frame.
>
> Is that right for 86-64?
>
> IIRC arguments always take (at least) 64bits.
> For any 32bit argument (register or stack) the high bits are undefined.
> (Maybe in kernel they are always zero?
> From 32bit userspace they are definitely random.)
>

Hello,

According to my testing, the compiler will always
pass the arguments on 8-byte size with "push" insn
if the count of the arguments that need to be passed
on stack more than 1 and the size of the argument
doesn't exceed 8-byte. In this case, there won't be
garbage. For example, the high 4-byte will be made 0
if the size of the argument is 4-byte, as the "push" insn
will copy the argument from regs or imm into stack
in 8-byte.

If the count of the arguments on-stack is 1 and its size
doesn't exceed 4-byte, some compiler, like clang, may
not use the "push" insn. Instead, it allocates 4 bytes in the
stack, and copies the arguments from regs or imm into
stack in 4-byte. This is the case we deal with here.

I'm not sure if I understand you correctly. Do you mean
that there will be garbage values for 32bit args?

> I think the called code is also responsible form masking 8 and 16bit
> values (in reality char/short args and return values just add code
> bloat).
>
> A 128bit value is either passed in two registers or two stack
> slots. If the last register is skipped it will be used for the
> next argument.
>

Yeah, this point is considered in save_args(). Once
this happen, the count of stack slots should more
then 1, and the arguments on-stack will be stored with
"push" insn in 8-byte. Therefore, there shouldn't be garbage
values in this case?

Do I miss something?

Thanks!
Menglong Dong

>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)