From: Andrea Claudi <aclaudi@redhat.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Stephen Hemminger <stephen@networkplumber.org>,
linux-netdev <netdev@vger.kernel.org>,
David Ahern <dsahern@gmail.com>,
asmadeus@codewreck.org
Subject: Re: [PATCH iproute2 v3 0/2] bpf: memory access fixes
Date: Mon, 25 May 2020 10:53:50 +0200 [thread overview]
Message-ID: <CAPpH65zTiy-9WxoK=JzUj2eR8pNu8Mf4xqMmmHtjVVfwTSgydA@mail.gmail.com> (raw)
In-Reply-To: <f18653bd-f9a2-8a87-49a5-f682038a8477@mojatatu.com>
On Sat, May 23, 2020 at 12:32 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2020-05-22 9:33 p.m., Daniel Borkmann wrote:
> > On 5/18/20 3:00 PM, Jamal Hadi Salim wrote:
> >> ping?
> >>
> >> Note: these are trivial bug fixes.
> >
> > Looking at c0325b06382c ("bpf: replace snprintf with asprintf when
> > dealing with long buffers"),
> > I wonder whether it's best to just revert and redo cleanly from
> > scratch.. How much testing has
> > been performed on the original patch? We know it is causing regressions,
> > and looking Jamal's
> > 2nd patch we do have patterns all over the place wrt error path that go
> > like:
>
> Reverting c0325b06382c would work as well..
>
> Note: I believe Andrea's original goal was to just get rid of a
> compiler warning from sprintf(). Stephen suggested to use
> asprintf. Andrea's original solution to get rid of the compiler
> warning would suffice. Maybe then an additional code audit to
> ensure consistency on usage of s[n]printf could be done and
> resolved separately.
>
Reverting c0325b06382c will for sure fix the segfault identified by
Jamal and get rid of the problems highlighted by Daniel and others.
To fix the s[n]printf truncation warning we can simply check for its
return value. From the snprintf man page:
"a return value of size or more means that the output was truncated."
(caveat: until glibc 2.0.6 ret value for truncation is -1)
Jamal: if this works for you, I can submit an alternative to this
patch series doing what I proposed above. What do you think?
Regards,
Andrea
> Thanks for taking the time Daniel.
>
> cheers,
> jamal
>
next prev parent reply other threads:[~2020-05-25 8:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-23 17:58 [PATCH iproute2 v3 0/2] bpf: memory access fixes Jamal Hadi Salim
2020-04-23 17:58 ` [PATCH iproute2 v3 1/2] bpf: Fix segfault when custom pinning is used Jamal Hadi Salim
2020-04-23 17:58 ` [PATCH iproute2 v3 2/2] bpf: Fix mem leak and extraneous free() in error path Jamal Hadi Salim
2020-04-24 16:58 ` [PATCH iproute2 v3 0/2] bpf: memory access fixes Andrea Claudi
2020-04-28 16:15 ` Jamal Hadi Salim
2020-05-18 13:00 ` Jamal Hadi Salim
2020-05-23 1:33 ` Daniel Borkmann
2020-05-23 10:32 ` Jamal Hadi Salim
2020-05-25 8:53 ` Andrea Claudi [this message]
2020-05-26 12:27 ` Jamal Hadi Salim
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPpH65zTiy-9WxoK=JzUj2eR8pNu8Mf4xqMmmHtjVVfwTSgydA@mail.gmail.com' \
--to=aclaudi@redhat.com \
--cc=asmadeus@codewreck.org \
--cc=daniel@iogearbox.net \
--cc=dsahern@gmail.com \
--cc=jhs@mojatatu.com \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.