| Message ID | 1537571127-10143-8-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | Misc Multipath patches | expand |
On Fri, 2018-09-21 at 18:05 -0500, Benjamin Marzinski wrote: > When get_vpd_sgio() finds out that the vpd info needed to be > truncated > to fit in the buffer, it doesn't trucate the size as well, which > allows > it to overwrite the buffer. Also, in once len is set to -ENODATA, > get_vpd_sgio() should exit, instead of using the negative len in > memcpy(). Found by coverity. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com> > --- > libmultipath/discovery.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c > index 0b1855d..3e0db7f 100644 > --- a/libmultipath/discovery.c > +++ b/libmultipath/discovery.c > @@ -1116,17 +1116,21 @@ get_vpd_sgio (int fd, int pg, char * str, int > maxlen) > return -ENODATA; > } > buff_len = get_unaligned_be16(&buff[2]) + 4; > - if (buff_len > 4096) > + if (buff_len > 4096) { > condlog(3, "vpd pg%02x page truncated", pg); > - > + buff_len = 4096; > + } > if (pg == 0x80) > len = parse_vpd_pg80(buff, str, maxlen); > else if (pg == 0x83) > len = parse_vpd_pg83(buff, buff_len, str, maxlen); > else if (pg == 0xc9 && maxlen >= 8) { > - len = buff_len < 8 ? -ENODATA : > - (buff_len <= maxlen ? buff_len : maxlen); > - memcpy (str, buff, len); > + if (buff_len < 8) > + len = -ENODATA; > + else { > + len = (buff_len <= maxlen)? buff_len : maxlen; > + memcpy (str, buff, len); > + } > } else > len = -ENOSYS; >
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index 0b1855d..3e0db7f 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -1116,17 +1116,21 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen) return -ENODATA; } buff_len = get_unaligned_be16(&buff[2]) + 4; - if (buff_len > 4096) + if (buff_len > 4096) { condlog(3, "vpd pg%02x page truncated", pg); - + buff_len = 4096; + } if (pg == 0x80) len = parse_vpd_pg80(buff, str, maxlen); else if (pg == 0x83) len = parse_vpd_pg83(buff, buff_len, str, maxlen); else if (pg == 0xc9 && maxlen >= 8) { - len = buff_len < 8 ? -ENODATA : - (buff_len <= maxlen ? buff_len : maxlen); - memcpy (str, buff, len); + if (buff_len < 8) + len = -ENODATA; + else { + len = (buff_len <= maxlen)? buff_len : maxlen; + memcpy (str, buff, len); + } } else len = -ENOSYS;
When get_vpd_sgio() finds out that the vpd info needed to be truncated to fit in the buffer, it doesn't trucate the size as well, which allows it to overwrite the buffer. Also, in once len is set to -ENODATA, get_vpd_sgio() should exit, instead of using the negative len in memcpy(). Found by coverity. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/discovery.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)