Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> 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
