I have checked received the following patch and received a feedback from a LLVM developer. On 07.11.2019 05:47, 'Dmitry Vyukov' via syzkaller-netbsd-bugs wrote: > I've consulted with some people and _presumably_ (to the degree one > can be sure about bitter corner cases of C/C++ :)) this is a correct > fix (and formally correct warnings from ubsan). > As 6.5.3.2/4 says, only &*p and &p[i] syntactic forms are defined as > special case of not being a dereference, but rather effectively an > address calculation. But &p->m is not. Thus it is interpreted as a > dereference that produces an lvalue and then taking address of that > lvalue. At the point of dereference we have UB. Your fix avoids the > dereference. There is another early crash on boot and I will fix it. [ 1.5079810] panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/vfs_subr.c:793:14, member access within null pointer of type 'struct vnode_impl' On 07.11.2019 00:03, Kamil Rytarowski wrote: > On 06.11.2019 23:38, Christos Zoulas wrote: >> On Nov 6, 11:17pm, n54%gmx.com@localhost (Kamil Rytarowski) wrote: >> -- Subject: Re: CVS commit: src/sys/kern >> >> | Technically, I think that this is a real UB. >> | >> | 6.3.2.3/7 >> | A pointer to an object type may be converted to a pointer to a >> | different object type. If the resulting pointer is not correctly >> | aligned for the referenced type, the behavior is undefined. >> >> Then you are right. I guess the rationale for the above is that >> ... pauses to think ... Dereferencing the new object with the >> different type can fail if the original pointer was unaligned? >> I don't see how. >> > > I recall a similar UB in tmux that is simpler to illustrate. > > struct screen *s = &data->screen; > > /// if data is NULL -> return > > format_add(ft, "selection_present", "%d", s->sel.flag); > > This triggered UBSan as we syntactically dereferenced a NULL pointer, > which is UB. The intention was to set s = data + offsetof(data, screen) > and very compiler optimizes it to do the right thing without real > dereference. > > Similarly from a syntactical point of view we first dereference > dlp->d_magic, and next ask for its address &(). This is how I understand > the GCC behavior here. > > This is certainly very sensitive behavior of the sanitzier, but it can > catch real bugs. It helped to diagnoze at least a single UVM crash (we > had crash reports from ASan and UBSan). > >> | I agree that this is appeasing the sanitizer. >> >> Yes, on that we agree. >> >> christos >> > > [1] > https://github.com/tmux/tmux/commit/8fb6666f1733ebd4dcb90ba01dbcfc750190c9df >
Attachment:
signature.asc
Description: OpenPGP digital signature