On Thu, Aug 08, 2024 at 10:55:19AM +0200, Martin Wilck wrote:
> On Wed, 2024-08-07 at 18:59 -0400, Benjamin Marzinski wrote:
> > The ontap prioritizer functions dump_cdb() and process_sg_error()
> > both
> > incorrectly set the snprintf() limits larger than the available
> > space.
> > Instead of multiplying the number of elements to print by the size of
> > an
> > element to calculate the limit, they multiplied the number of
> > elements
> > to print by the maximum number of elements that the buffer could
> > hold.
> >
> > Fix this by making these functions use strbufs instead.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> > ---
> >
> > changes in v2:
> > Use strbufs as suggested by Martin Wilck.
> >
> > libmultipath/prioritizers/ontap.c | 28 +++++++++++++++-------------
> > 1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/libmultipath/prioritizers/ontap.c
> > b/libmultipath/prioritizers/ontap.c
> > index 117886ea..b508371d 100644
> > --- a/libmultipath/prioritizers/ontap.c
> > +++ b/libmultipath/prioritizers/ontap.c
> > @@ -23,6 +23,7 @@
> > #include "prio.h"
> > #include "structs.h"
> > #include "unaligned.h"
> > +#include "strbuf.h"
> >
> > #define INQUIRY_CMD 0x12
> > #define INQUIRY_CMDLEN 6
> > @@ -35,32 +36,33 @@
> > static void dump_cdb(unsigned char *cdb, int size)
> > {
> > int i;
> > - char buf[10*5+1];
> > - char * p = &buf[0];
> > + STRBUF_ON_STACK(buf);
> >
> > - condlog(0, "- SCSI CDB: ");
> > - for (i=0; i<size; i++) {
> > - p += snprintf(p, 10*(size-i), "0x%02x ", cdb[i]);
> > + for (i = 0; i < size; i++) {
> > + if (print_strbuf(&buf, "0x%02x ", cdb[i]) < 0) {
> > + condlog(0, "ontap prio: failed to dump SCSI
> > CDB");
>
> If print_strbuf() fails (very probably because it failed to allocate
> memory), condlog() is unlikely to succeed, either. I thought we agreed
> that trying to spit out error messages in this situation makes little
> sense.
>
> Anyway, I guess the condlog call doesn't actually hurt, so:
>
> Reviewed-by: Martin Wilck <mwilck@xxxxxxxx>
>
> (If you consent, I'll edit the commit and remove the condlog call).
Feel free to remove them.
-Ben
>
> Regards,
> Martin
