MIME-Version: 1.0
In-Reply-To: <CAHk-=wjy_q9t4APgug9q-EBMRKAybXt9DQbyM9Egsh=F+0k2Mg@mail.gmail.com>
References: <CAHk-=wgbm1rjkSs0w+dVJJzzK2M1No=j419c+i7T4V4ky2skOw@mail.gmail.com>
 <20230302083025.khqdizrnjkzs2lt6@wittgenstein> <CAHk-=wivxuLSE4ESRYv_=e8wXrD0GEjFQmUYnHKyR1iTDTeDwg@mail.gmail.com>
 <CAGudoHF9WKoKhKRHOH_yMsPnX+8Lh0fXe+y-K26mVR0gajEhaQ@mail.gmail.com>
 <ZADoeOiJs6BRLUSd@ZenIV> <CAGudoHFhnJ1z-81FKYpzfDmvcWFeHNkKGdr00CkuH5WJa2FAMQ@mail.gmail.com>
 <CAHk-=wjp5fMupRwnROtC5Yn+MVLA7v=J+_QJSi1rr3qAjdsfXw@mail.gmail.com>
 <CAHk-=wi11ZbOBdMR5hQDz0x0NNZ9gM-4SxXxK-7R3_yh7e10rQ@mail.gmail.com>
 <ZAD21ZEiB2V9Ttto@ZenIV> <6400fedb.170a0220.ece29.04b8@mx.google.com>
 <ZAEC3LN6oUe6BKSN@ZenIV> <CAG_fn=UQEuvJ9WXou_sW3moHcVQZJ9NvJ5McNcsYE8xw_WEYGw@mail.gmail.com>
 <CAGudoHFqNdXDJM2uCQ9m7LzP0pAx=iVj1WBnKc4k9Ky1Xf5XmQ@mail.gmail.com>
 <CAHk-=wh-eTh=4g28Ec5W4pHNTaCSZWJdxVj4BH2sNE2hAA+cww@mail.gmail.com>
 <CAGudoHG+anGcO1XePmLjb+Hatr4VQMiZ2FufXs8hT3JrHyGMAw@mail.gmail.com> <CAHk-=wjy_q9t4APgug9q-EBMRKAybXt9DQbyM9Egsh=F+0k2Mg@mail.gmail.com>
From:   Mateusz Guzik <mjguzik@gmail.com>
Date:   Fri, 3 Mar 2023 21:39:11 +0100
Message-ID: <CAGudoHGYaWTCnL4GOR+4Lbcfg5qrdOtNjestGZOkgtUaTwdGrQ@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Alexander Potapenko <glider@google.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Kees Cook <keescook@chromium.org>,
        Eric Biggers <ebiggers@google.com>,
        Christian Brauner <brauner@kernel.org>, serge@hallyn.com,
        paul@paul-moore.com, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 3/3/23, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Fri, Mar 3, 2023 at 11:37=E2=80=AFAM Mateusz Guzik <mjguzik@gmail.com>=
 wrote:
>>
>> I mentioned in the previous e-mail that memset is used a lot even
>> without the problematic opt and even have shown size distribution of
>> what's getting passed there.
>
> Well, I *have* been pushing Intel to try to fix memcpy and memset for
> over two decades by now, but with FSRM I haven't actually seen the
> huge problems any more.
>

rep *stos* remains crap with FSRM, but I don't have sensible tests
handy nor the ice lake cpu i tested on at the moment

> I actually used to have the reverse of your hack for this - I've had
> various hacks over the year that made memcpy and memset be inlined
> "rep movs/stos", which (along with inlined spinlocks) is a great way
> to see the _culprit_ (without having to deal with the call chains -
> which always get done the wrong way around imnsho).
>

that's all hackery which makes sense pre tooling like bpftrace, people
can do better now (see the second part of the email)

I think there is a systemic problem which comes with the kzalloc API, consi=
der:
static struct file *__alloc_file(int flags, const struct cred *cred)
{
	struct file *f;
	int error;

	f =3D kmem_cache_zalloc(filp_cachep, GFP_KERNEL);
	if (unlikely(!f))
		return ERR_PTR(-ENOMEM);
        [bunch of the struct gets initialized here]

the allocation routine does not have any information about the size
available at compilation time, so has to resort to a memset call at
runtime. Instead, should this be:

f =3D kmem_cache_alloc(...);
memset(f, 0, sizeof(*f));

... the compiler could in principle inititalize stuff as indicated by
code and emit zerofill for the rest. Interestingly, last I checked
neither clang nor gcc knew how to do it, they instead resort to a full
sized memset anyway, which is quite a bummer.

Personally i grew up on dtrace, bpftrace I can barely use and don't
know how to specifically get the caller, but kstack(2) seems like a
good enough workaround.

as an example here is a one-liner to show crappers which do 0-sized ops:
bpftrace -e 'kprobe:memset,kprobe:memcpy /arg2 =3D=3D 0/ { @[probe,
kstack(2)] =3D count(); }'

one can trace all kinds of degeneracy like that without recompiling
anything, provided funcs are visible to bpftrace

sample result from the above one-liner while doing 'make clean' in the
kernel dir:
@[kprobe:memcpy,
    memcpy+5
    realloc_array+78
]: 1
@[kprobe:memcpy,
    memcpy+5
    push_jmp_history+125
]: 1
@[kprobe:memset,
    memset+5
    blk_mq_dispatch_rq_list+687
]: 3
@[kprobe:memcpy,
    memcpy+5
    mix_interrupt_randomness+192
]: 4
@[kprobe:memcpy,
    memcpy+5
    d_alloc_pseudo+18
]: 59
@[kprobe:memcpy,
    memcpy+5
    add_device_randomness+111
]: 241
@[kprobe:memcpy,
    memcpy+5
    add_device_randomness+93
]: 527
@[kprobe:memset,
    memset+5
    copy_process+2904
]: 2054
@[kprobe:memset,
    memset+5
    dup_fd+283
]: 6162

--=20
Mateusz Guzik <mjguzik gmail.com>