Re: Concerns in kern/subr_kobj.c

[christos%astron.com@localhost (Christos Zoulas)]

In article <Pine.NEB.4.64.1704171932080.13026%speedy.whooppee.com@localhost>,
Paul Goyette  <paul%whooppee.com@localhost> wrote:
>-=-=-=-=-=-
>
>On Mon, 17 Apr 2017, Paul Goyette wrote:
>
>> While perusing the code, I noticed some possible issues:
>>
>> 1. In kobj_unload() the calls to kobj_machdep() for data and rodata
>>   sections are conditional on the appropriate ko->ko_xxx_address being
>>   non-zero, yet the corresponding call for the text section is
>>   unconditional.
>>
>>   But, from code just a little bit further down, there is a check
>>   for the ko->ko_text_address being 0 before calling uvm_km_free()
>>   on the text section.
>>
>>   It seems to me that both calls should be conditional, or both
>>   should be unconditional.
>>
>> 2. In kobj_affix(), the calls to kobj_machdep() for data and rodata
>>   sections discard any possible error value from previous sections.
>>   So, if kobj_machdep() fails for the text section, but succeeds for
>>   the data or rodata sections, the rest of the code is unaware of
>>   any error having occurred.
>>
>>   In particular, the trailing call to kobj_unload() would not happen;
>>   instead the code would simply proceed to update the VM protection
>>   bits for the text and rodata sections.
>
>I'm proposing the attached diffs to address these issues.  Does anyone 
>see any reason why these should not be committed?

How about if (error == 0 && kobj->ko_foo_address) to avoid the extra
level of indentation?

christos