[dm-devel] [PATCH 05/21] libmultipath (coverity): improve input checking in parse_vpd_pg83

[Martin Wilck]

On Mon, 2021-11-29 at 19:17 -0600, Benjamin Marzinski wrote:
> On Fri, Nov 19, 2021 at 12:13:22AM +0100, mwilck at suse.com wrote:
> > From: Martin Wilck <mwilck at suse.com>
> > 
> > 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?

Martin