Re: [PATCH v7 08/12] xen: add /buildinfo/config entry to hypervisor filesystem

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Apr 28, 2020, at 9:39 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 28.04.2020 10:24, George Dunlap wrote:
>>> On Apr 28, 2020, at 8:20 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 27.04.2020 18:25, George Dunlap wrote:
>>>> If Jan is OK with it simply being outside CONFIG_EXPERT, then great.  But 
>>>> if he insists on some kind of testing for it to be outside of 
>>>> CONFIG_EXPERT, then again, the people who want it to be security supported 
>>>> should be the ones who do the work to make it happen.
>>> 
>>> I don't understand this part, I'm afraid: Without a config option,
>>> the code is going to be security supported as long as it doesn't
>>> get marked otherwise (experimental or what not). With an option
>>> depending on EXPERT, what would become security unsupported is the
>>> non-default (i.e. disabled) setting. There's not a whole lot to
>>> test there, it's merely a formal consequence of our general rules.
>>> (Of course, over time dependencies of other code may develop on
>>> the information being available e.g. to Dom0 userland. Just like
>>> there's Linux userland code assuming the kernel config is
>>> available in certain ways [I don't necessarily mean the equivalent
>>> of hypfs here], to then use it in what I'd call abusive ways in at
>>> least some cases.)
>> 
>> Here’s an argument you might make:
>> 
>> “As a member of the security team, I don’t want to be on the hook for 
>> issuing XSAs for code which isn’t at least smoke-tested.  Therefore, I 
>> oppose any patch adding CONFIG_HYPFS outside of CONFIG_EXPERT, *unless* 
>> there is a concrete plan for getting regular testing for CONFIG_HYPFS=n.”
>> 
>> I’m not saying that’s an argument you *should* make.  But personally I don’t 
>> have a strong argument against such an argument. So, it seems to me, if you 
>> did make it, you have a reasonable chance of carrying your point.
>> 
>> Now consider this hypothetical universe where you made that argument and 
>> nobody opposed it.  In order to get a particular feature (CONFIG_HYPFS=n 
>> security supported), there is extra work that needs to be done (getting 
>> CONFIG_HYPFS=n tested regularly).  My point was, the expectation should be 
>> that the extra work will be done by the people who want or benefit from the 
>> feature; the series shouldn’t be blocked until Juergen implements 
>> CONFIG_HYPFS=n testing (since he doesn’t personally have a stake in that 
>> feature).
>> 
>> Now obviously, doing work to help someone else out in the community is of 
>> course a good thing to do; it builds goodwill, uses our aggregate resources 
>> more efficiently, and makes our community more enjoyable to work with. But 
>> the goodwill primarily comes from the fact that it was done as a voluntary 
>> choice, not as a requirement.
>> 
>> Juergen was balking at having to do what he saw as extra work to implement 
>> CONFIG_HYPFS.  I wanted to make it clear that even though I see value in 
>> having CONFIG_HYPFS, *he* doesn’t have to do the work if he doesn’t want to 
>> (although it would certainly be appreciated if he did).  And this paragraph 
>> was extending the same principle into the hypothetical universe where 
>> someone insisted that CONFIG_HYPFS=n had to be tested before being security 
>> supported.
>> 
>> Hope that makes sense. :-)
> 
> Yes, it does, thanks for the clarification. I can see what you describe
> as a valid perspective to take, but really in my request to Jürgen I
> took another: Now that we have Kconfig, additions of larger bodies of
> code (possibly also just in terms of binary size) should imo generally
> be questioned whether they want/need to be built for everyone. I.e. it
> is not to be left to people being worried about binary sizes to arrange
> for things to not be built, but for people contributing new but not
> entirely essential code to consider making it option from the very
> beginning.

I think that’s a reasonable position to take, but needs to be balanced on the 
amount of work that this would actually require.  If it only requires adding a 
handful of #ifdef’s and maybe making a few stubs, then yes, asking the 
submitter to make the change makes sense.  But if it requires three dozen 
#ifdef’s throughout the code and a fairly major architectural change, then I 
think it’s reasonable for a submitter to push back.

I don’t really understand why Juergen thinks adding CONFIG_HYPFS would cause a 
lot of code churn; my argumentation here is based on the assumption that his 
assessment is correct.

 -George