[PATCH 00/14] Make the user mode driver code a better citizen

Eric W. Biederman ebiederm at xmission.com
Mon Jun 29 20:19:59 UTC 2020


Tetsuo Handa <penguin-kernel at i-love.sakura.ne.jp> writes:

> On 2020/06/29 4:44, Alexei Starovoitov wrote:
>> But all the defensive programming kinda goes against general kernel style.
>> I wouldn't do it. Especially pr_info() ?!
>> Though I don't feel strongly about it.
>
> Honestly speaking, caller should check for errors and print appropriate
> messages. info->wd.mnt->mnt_root != info->wd.dentry indicates that something
> went wrong (maybe memory corruption). But other conditions are not fatal.
> That is, I consider even pr_info() here should be unnecessary.

They were all should never happen cases.  Which is why my patches do:
if (WARN_ON_ONCE(...))

That let's the caller know the messed up very clearly while still
providing a change to continue.

If they were clearly corruption no ones kernel should ever continue
BUG_ON would be appropriate.

>> I would like to generalize elf_header_check() a bit and call it
>> before doing blob_to_mnt() to make sure that all blobs are elf files only.
>> Supporting '#!/bin/bash' or other things as blobs seems wrong to me.

I vote for not worry about things that have never happened, and are
obviously incorrect.

The only points of checks like that is to catch cases where other
developers misunderstand the interface.  When you get to something like
sysfs with lots and lots of users where it is hard to audit there
is real value in sanity checks.  In something like this with very few
users. Just making the code clear should be enough for people not to do
ridiculous things.


In any case Tetsuo I will leave futher sanity checks for you and Alexei
to work out.  It is beyond the scope of my patchset, and they are easy
enough to add as follow on patches.

Eric



More information about the Linux-security-module-archive mailing list