bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Florent Revest <revest@chromium.org>
Cc: bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	KP Singh <kpsingh@kernel.org>,
	Brendan Jackman <jackmanb@google.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next 0/2] Implement BPF formatted output helpers with bstr_printf
Date: Fri, 23 Apr 2021 16:31:54 +0200	[thread overview]
Message-ID: <8ef7319d-7692-0067-bb86-a0d5465e997f@rasmusvillemoes.dk> (raw)
In-Reply-To: <CABRcYmLDBfoM8rOwPf+SdqkmJgtFRLYF94S4Fv2eU=Uwg4asTQ@mail.gmail.com>

On 23/04/2021 15.26, Florent Revest wrote:
> On Fri, Apr 23, 2021 at 10:50 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:

>>> This solves a bug reported by Rasmus Villemoes that would mangle
>>> arguments on 32 bit machines.
>> That's not entirely accurate. The arguments are also mangled on x86-64,
>> it's just that in a few cases that goes unnoticed. That's why I
>> suggested you try and take your test case (which I assume had been
>> passing with flying colours on x86-64) and rearrange the specifiers,
>> arguments and expected output string so that the (morally) 32 bit
>> arguments end up beyond those-that-end-up-in-the-reg_save_area.
>> IOWs, it is the 32 bit arguments that are mangled (because they get
>> passed as-if they were actually 64 bits), and that applies on all
>> architectures; nothing to do with sizeof(long).
> Mh, yes, I get your point and I agree that my description does not
> really fit what you reported.
> I tried what you suggested though, with the current bpf-next/master on x86_64:
> BPF_SNPRINTF(out, sizeof(out),
> "%u %d %u %d %u %d %u %d %u %d %u %d",
> 1, -2, 3, -4, 5, -6, 7, -8, 9, -10, 11, -12);
> And out is "1 -2 3 -4 5 -6 7 -8 9 -10 11 -12" so i can't seem to be
> able to produce the bug you described.
> Do you think I'm missing something? Would you try it differently ?

Nah, sorry, I must have misremembered the x86-64 ABI. Re-reading it, it
clearly says as the very first thing "The size of each argument gets
rounded up to eightbytes". So each of the ints that get passed on the
stack do indeed occupy 8 bytes (i.e., the overflow_area pointer gets
adjusted by 8 bytes, for both va_arg(ap, int) and va_arg(ap, long)). So
it will indeed work on x86-64. And probably other 64 bit ABIs behave the
same way (it would make sense) - at least ppc64 and arm64 seem to behave
like that.

So in a round-about way it's probably true that the bug would only be
seen on 32 bit machines, but only because all (relevant) 64 bit arches
seem to, on the ABI level, effectively do argument promotion to u64 anyway.


      reply	other threads:[~2021-04-23 14:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23  1:15 [PATCH bpf-next 0/2] Implement BPF formatted output helpers with bstr_printf Florent Revest
2021-04-23  1:15 ` [PATCH bpf-next 1/2] seq_file: Add a seq_bprintf function Florent Revest
2021-04-23  1:15 ` [PATCH bpf-next 2/2] bpf: Implement formatted output helpers with bstr_printf Florent Revest
2021-04-23  9:27   ` Rasmus Villemoes
2021-04-23 13:45     ` Florent Revest
2021-04-23  8:50 ` [PATCH bpf-next 0/2] Implement BPF " Rasmus Villemoes
2021-04-23 13:26   ` Florent Revest
2021-04-23 14:31     ` Rasmus Villemoes [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ef7319d-7692-0067-bb86-a0d5465e997f@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jackmanb@google.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=revest@chromium.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).