On 07.11.2019 18:20, Martin Husemann wrote:
> On Thu, Nov 07, 2019 at 06:08:40PM +0100, Kamil Rytarowski wrote:
>> Please review:
>>
>> http://netbsd.org/~kamil/patch-00194-disklabel-alignment.txt
>>
>> This patch works for me.
>
> Yes, I believe that it does - but why is it needed?
>
syzbot reported on boot:
member access within misaligned address 0xffff942d3de8c03c for type
'const struct disklabel' which requires 8 byte alignment
> dlp = (void *)a->bp->b_data;
>
> Here we can assume that b_data is properly aligned (likely even 512 byte
> aligned or similar).
>
I just have got this backtrace:
[ 1.3496393] panic: UBSan: Undefined Behavior in
/syzkaller/managers/netbsd-kubsan/kernel/sys/kern/subr_disk_mbr.c:574:9,
member access within misaligned address 0xffff942d3de8c03c for type
'const struct disklabel' which requires 8 byte alignment
[ 1.3663828] cpu0: Begin traceback...
[ 1.3689943] vpanic() at netbsd:vpanic+0x2aa sys/kern/subr_prf.c:336
[ 1.3788101] isAlreadyReported() at netbsd:isAlreadyReported
[ 1.3988418] HandleTypeMismatch.part.1() at
netbsd:HandleTypeMismatch.part.1+0xcc
[ 1.4088591] HandleTypeMismatch() at netbsd:HandleTypeMismatch+0x7b
sys/../common/lib/libc/misc/ubsan.c:408
[ 1.4288896] check_label_magic() at netbsd:check_label_magic+0x9f
sys/kern/subr_disk_mbr.c:574
[ 1.4389098] validate_label() at netbsd:validate_label+0x1b4
sys/kern/subr_disk_mbr.c:626
[ 1.4489214] look_netbsd_part() at netbsd:look_netbsd_part+0x3ce
sys/kern/subr_disk_mbr.c:526
[ 1.4689530] scan_mbr() at netbsd:scan_mbr+0x24a
sys/kern/subr_disk_mbr.c:234
[ 1.4789689] readdisklabel() at netbsd:readdisklabel+0x412
sys/kern/subr_disk_mbr.c:448
[ 1.4898443] dk_getdisklabel() at netbsd:dk_getdisklabel+0x192
sys/dev/dksubr.c:931
[ 1.5090177] dk_open() at netbsd:dk_open+0x456 sys/dev/dksubr.c:177
[ 1.5190333] sdopen() at netbsd:sdopen+0x114 sys/dev/scsipi/sd.c:543
[ 1.5290486] cdev_open() at netbsd:cdev_open+0xe5
sys/kern/subr_devsw.c:871
[ 1.5490803] spec_open() at netbsd:spec_open+0x3ad
sys/miscfs/specfs/spec_vnops.c:562
[ 1.5591508] VOP_OPEN() at netbsd:VOP_OPEN+0x113 sys/kern/vnode_if.c:298
[ 1.5791291] dkwedge_discover() at netbsd:dkwedge_discover+0xcf
sys/dev/dkwedge/dk.c:931
[ 1.5891449] sdattach() at netbsd:sdattach+0x53f sys/dev/scsipi/sd.c:362
[ 1.5991611] config_attach_loc() at netbsd:config_attach_loc+0x432
sys/kern/subr_autoconf.c:1604
[ 1.6191925] scsi_probe_bus() at netbsd:scsi_probe_bus+0xad8
scsi_probe_device sys/dev/scsipi/scsiconf.c:1035 [inline]
[ 1.6191925] scsi_probe_bus() at netbsd:scsi_probe_bus+0xad8
sys/dev/scsipi/scsiconf.c:413
[ 1.6292092] scsibus_discover_thread() at
netbsd:scsibus_discover_thread+0xdd scsibus_config
sys/dev/scsipi/scsiconf.c:320 [inline]
[ 1.6292092] scsibus_discover_thread() at
netbsd:scsibus_discover_thread+0xdd sys/dev/scsipi/scsiconf.c:285
[ 1.6392253] cpu0: End traceback...
[ 1.6392253] fatal breakpoint trap in supervisor mode
[ 1.6392253] trap type 1 code 0 rip 0xffffffff8021dddd cs 0x8 rflags
0x282 cr2 0 ilevel 0 rsp 0xffffbc80a5c02280
[ 1.6555812] curlwp 0xffff942c2bcce9c0 pid 0.28 lowest kstack
0xffffbc80a5bff2c0
https://syzkaller.appspot.com/bug?extid=56769dece0ec3e35731e
> for (;; dlp = (void *)((char *)dlp + sizeof(uint32_t))) {
>
> ugly as it is written, this still should ensure proper (4 byte) alignement
> of all later values of dlp.
We actually need 8-byte alignment to make a compiler appeased.. so this
patch is still good (if I remember correctly it was the original
motivation for patching the userland part).
Can I land it?
>
> Do we have any architectures with odd LABELOFFSET?
> I only found 0, 64, 128 and 516, none of those should cause alignment
> issues in the code following.
>
> So where is the problem?
>
Could that be a half-corrupted image after some crash? The previous
patch actually fixed booting.. so I don't know.
> Martin
>
Attachment:
signature.asc
Description: OpenPGP digital signature