Re: CVS commit: src/sys/kern

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Thu, Nov 07, 2019 at 18:08:40 +0100, Kamil Rytarowski wrote:

> On 07.11.2019 16:45, Kamil Rytarowski wrote:
> > On 07.11.2019 16:26, Martin Husemann wrote:
> >> On Thu, Nov 07, 2019 at 02:53:08PM +0100, Kamil Rytarowski wrote:
> >>> On 07.11.2019 14:25, Valery Ushakov wrote:
> >>>> If the sanitizer does complain about other uses, there is little point
> >>>> in fixing one instance and not the others.
> >>>
> >>> We already agreed with Christos that this is appeasing of GCC. If you
> >>> want to scan the whole kernel (or whole C) file for more occurrences of
> >>> violations - please go for it.
> >>
> >> No. The commit needs to be reverted, and then
> >>
> >>  a) either the root cause for the unaligned address be fixed or
> >>  b) some other means be found to make the sanitizer shut up
> >>
> >> As uwe said: papering over a tiny detail that *never* hits in the real
> >> world but potentialy hiding a real issue is not the way to go.
> >>
> > 
> > I don't have a readily available reproducer locally but it was breaking
> > syzbot from booting after the switch to gcc8. I will fix it differently
> > aligning the whole struct (so the same approach as we use in userland)
> > and backout this change.
> > 
> 
> Please review:
> 
> http://netbsd.org/~kamil/patch-00194-disklabel-alignment.txt
> 
> This patch works for me.

What happens if you change check_label_magic() to use direct member
accesses (as the code did before xtos change it) instead of memcmp?
Does that shup up the sanitizer?  I assume it should as it doesn't
complain about other member accesses.  I'd strongly prefer this change
for now.

-uwe