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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 30, 2021 at 09:14:10PM +0100, Martin Wilck wrote:
> On Tue, 2021-11-30 at 11:30 -0600, Benjamin Marzinski wrote:
> > 
> > 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.
> 
> That's what my code does (embarrassingly, I didn't realize that when I
> wrote my previous reply). 
> 
> In shortened pseudo code:
> 
> 	int prio = -1;
> 	int err = -ENODATA;
> 
> 	d = first_designator();
> 	while (d <= in + in_len - 4) {
> 		bool invalid = false;
> 		int new_prio = -1;
> 
>                 if (designator_is_invalid(d))
>                      invalid = true;
>                 else if (lun_association(d))
>                      new_prio = prio(d);
> 
> 	next_designator:
> 		if (invalid) {
> 			err = -EINVAL;
> 			break;  /** see below **/
> 		}
> 		if (new_prio > prio) {
> 			vpd = d;
> 			prio = new_prio;
> 		}
> 		d = next_designator(d);
> 	}
> 
>         /* if we did find a valid designator, prio will be > 0, and we
>            will not error out */
> 	if (prio <= 0)
> 		return err;
> 
>         convert_designator_to_wwid(d);
> 
> If you think we should use a different strategy, please explain.
> We *could* try to go on even after encountering broken designators,
> assuming the length field is correct if it's within the given limits,
> even if the rest is bogus. So baiscally instead of the break statement
> above, we'd continue the loop.
> 
> Would you prefer that?

Again looking at sg_inq, it will loop through all the descriptors,
trusting the length field, until it gets to the whole data length. It
prints a warning if it doesn't end up at exactly the data length, but
still exports all the IDs it finds.  If an individual descriptor doesn't
make sense, it gets skipped. That would be my preference.

-Ben

> Regards,
> Martin

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux