Re: [RFC PATCH] debug: Add tests for fortified fcntl ()

[Sergey Bugaev <bugaevc@gmail.com>]

Hello,

thank you for taking a look.

On Tue, May 23, 2023 at 9:40 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
> > +  res = __fcntl (lockfd1, F_OFD_GETLK, &flock);
>
> This is an external case, so use the external default name.

Do you mean that this should be s/__fcntl/fcntl/?

If that is what you mean -- here's why I used __fcntl here, but not in
all the other places:

The point of the OFD tests in the patch is to check for 32-bit vs
64-bit confusion in the fortification. If, for example, in some
configuration the plain fcntl is supposed to be 32-bit (and struct
flock is 32-bit), but the fortified version of fcntl mistakenly
forwards to fcntl64, then the 32-bit struct flock will not be
converted to struct flock64, and will be passed to the kernel as-is.
It will yield some garbage when interpreted as a struct flock64, and
the kernel will likely reject it with an error (EINVAL, perhaps) --
which is what the test checks against.

But the test also needs to work on the kernel versions that do not
support OFD locks, so it does this check for support being present at
all first, and skips running the OFD tests if the kernel does not
support it. But how do you check for OFD locks support? -- you try to
F_OFD_GETLK and see if the kernel returns EINVAL. If it does return
that, OFD locks are unsupported.

And herein lies the problem, we cannot differentiate between EINVALs
caused by missing kernel support from EINVALs caused by the very kind
of bugs this test checks against! We'd mistake the actual issue we
wanted to catch for missing kernel support, skip the bulk of the test,
and report success.

So in this first check of whether or not OFD locks are supported at
all, I've used __fcntl, which does not get fortified and so cannot
suffer from the 32-bit vs 64-bit confusion bug. The rest of the code
uses the regular fcntl. There's a comment above briefly mentioning
this, too -- clearly it has to be expanded.

So, with this in mind, should I be using __fcntl here, or is there a
better option?

Sergey