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).
Regards,
Martin
