On Tue, Nov 30, 2021 at 12:21:44PM +0100, Martin Wilck wrote: > On Mon, 2021-11-29 at 19:17 -0600, Benjamin Marzinski wrote: > > On Fri, Nov 19, 2021 at 12:13:22AM +0100, mwilck@xxxxxxxx wrote: > > > From: Martin Wilck <mwilck@xxxxxxxx> > > > > > > Check offsets and other obvious errors in the VPD83 data. > > > > > > The original reason to do this was to fix "tained scalar" > > > warnings from coverity. But this doesn't suffice for coverity > > > without using a constant boundary (WWID_SIZE) for "len" in > > > parse_vpd_pg80() and for "vpd_len" in parse_vpd_pg83(), even > > > though the computed boundaries are tighter than the constant > > > ones. > > > > > > > This looks fine, but I do wonder if we are being too strict. I'm not > > sure we should be failing, if sg_inq wouldn't fail. The goal of the > > fallback code is to get the WWID the udev would have gotten, and > > being > > lenient with scsi devices that don't quite follow the standard is > > usually the best policy. Thoughts? > > If we encounter a broken VPD entry, IMO we can't continue reading > further entries, because if the entry is broken, we can't trust the > length value which is part of the entry. We may be reading total junk > if we follow it. > > We might simply discard the broken entry, stop iterating over the > designators, see if we got a usable designator up to that point (i.e., > previously), and if yes, use that entry, printing a big fat warning. > That means we'd ultimately fail only if there was no usable designator > before the broken one. > > Would you prefer that strategy? If you export scsi IDs from sg_inq, it will print them out until it hits an error, so I would prefer if we used the best designator we got before we hit the error, to match it. -Ben > > Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
