* Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Wed, Mar 4, 2015 at 2:16 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> > On Wed, Mar 04, 2015 at 12:00:37AM -0800, Yinghai Lu wrote:
> >> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> >> is using address as value for kaslr_enabled.
> >>
> >> That will random kaslr_enabled get that set or cleared.
> >> Will have problem for system really have kaslr enabled.
> >>
> >> -v2: update changelog.
> >
> > This is still not good enough. Please do this:
> >
> > In commit f47233c2d34f we did A. The problem with that is B. Change the
> > code to do C.
> >
> > Now you only have to fill out the A,B and C variables with the
> > respective text which is understandable even for people who don't know
> > this code.
> >
>
> I don't know, that is trivial and obvious.
The fix might be obvious, the effects of the bug are not obvious at
all, as you yourself show that you don't understand your own change,
which is evident from the changelog you've written:
> Please check if it is ok:
>
> Subject: [PATCH v3] x86, kaslr: get kaslr_enabled back correctly
Missing capitalization.
>
> commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
> is using address as value for kaslr_enabled.
Missing capitalization. Do you really expect maintainers to fix up
every single sentence of yours??
> That will get wrong value for kaslr_enabled, so have problem for
> system really have kaslr enabled.
This sentence does not parse, nor is it correct: the bug isn't just
triggering on systems that want to have kaslr enabled - but also on
bootloaders that happen to pass in a kaslr boot parameter but have the
switch value disabled...
You also need to point out the important fact that bootloaders that
don't try to use the kaslr extension (i.e. that don't use SETUP_KASLR)
work just fine - this is why the bug was not noticed to begin with,
i.e. the overwhelming majority of systems out there.
> This patch change to using early map and accessing the value.
s/change to using/changes the code to use/
It is totally unacceptable that you don't do proper analysis of the
patches you submit, and that you don't bother writing proper, readable
changelogs.
Your flippant "that is trivial and obvious" attitude towards
changelogs is unacceptable as well. And this is not about English
knowledge: missing capitalization is a very simple concept any
beginning coder should be able to graps the first time it's pointed
out... yet for the past 3 years half of your patches had totally
careless, often unreadable changelogs.
These subpar changelogs and patches show plain laziness, sloppiness
and lack of care to write clean changelogs - and that sloppiness not
only makes it much harder for maintainers to process your patches, but
also tends to creep over into your patches as well, causing repeat
problems again and again...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
