Re: [PATCH 5/6] libmultipath: Fix sgio_get_vpd()

[Martin Wilck <mwilck@suse.com>]

On Mon, 2018-03-05 at 17:53 +0100, Martin Wilck wrote:
> On Thu, 2018-03-01 at 11:29 -0800, Bart Van Assche wrote:
> > Pass the VPD page number to sgio_get_vpd() such that the page
> > needed
> > by the caller is queried instead of page 0x83. Fix the statement
> > that
> > computes the length of the page returned by do_inq(). Fix the
> > return
> > code check in the caller of sgio_get_vpd().
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> > ---
> 
> Bart, thanks for the patch. Please consider rebasing your work on my
> previously submitted patches which have already been positively
> reviewed ([1], [2]):
> 
>  libmultipath: get_vpd_sgio: support VPD 0xc9
>  libmultipath: sgio_get_vpd: add page argument
>  libmultipath: fix return code of sgio_get_vpd()
> 
> Otherwise, ACK.

Sorry, I found a glitch.

With the change of the "len" calculation and the return value check, we
  need to make sure sufficient data has been read:

@@ -1100,13 +1101,19 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 	unsigned char buff[4096];
 
 	memset(buff, 0x0, 4096);
-	if (sgio_get_vpd(buff, 4096, fd, pg) <= 0) {
+	buff_len = sgio_get_vpd(buff, 4096, fd, pg);
+	if (buff_len < 0) {
 		condlog(3, "failed to issue vpd inquiry for pg%02x",
 			pg);
-		return -errno;
+		return errno != 0 ? -errno : -1;
 	}
 
-	if (buff[1] != pg) {
+	if (buff_len < 4) {
+		condlog(3, "vpd pg%02x error, short read", pg);
+		return -ENODATA;
+	}
+
+	if ( buff[1] != pg) {
 		condlog(3, "vpd pg%02x error, invalid vpd page %02x",
 			pg, buff[1]);
 		return -ENODATA;

As you can see I added also a small hunk that makes it explcicit that
we don't return 0 on failure by accident.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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