* Re: multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len
2014-04-05 11:13 multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len Sebastian Herbszt
@ 2014-05-21 11:14 ` Sebastian Herbszt
2014-05-21 22:46 ` Sebastian Herbszt
2014-05-21 20:31 ` Christophe Varoqui
2014-06-10 5:58 ` Christophe Varoqui
2 siblings, 1 reply; 5+ messages in thread
From: Sebastian Herbszt @ 2014-05-21 11:14 UTC (permalink / raw)
To: dm-devel; +Cc: Sebastian Herbszt
> Hello,
>
> while playing with multipath-tools and scst I got the following warning:
>
> [ 6382.467569] [3806]: scst: scst_parse_cmd:826:Warning: expected
> transfer length 256 for opcode 0x12 (handler vdisk_blockio, target
> scst_local) doesn't match decoded value 128
> [ 6382.467574] [3806]: scst_parse_cmd:828:Suspicious CDB:
> [ 6382.467579] (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
> [ 6382.467581] 0: 12 01 c0 00 80 00 ......
>
> This happened when my scst target pretended to be a CLARiiON.
> I noticed a mismatch in emc_clariion_prio (libmultipath/prioritizers/emc.c)
> between the allocation length and dxfer_len:
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> unsigned char sense_buffer[256];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> sizeof(sb), 0};
>
> ...
>
> memset(&sense_buffer, 0, 256);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
> io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
> io_hdr.dxfer_len = sizeof (sense_buffer);
> io_hdr.dxferp = sense_buffer;
> io_hdr.cmdp = inqCmdBlk;
> io_hdr.sbp = sb;
> io_hdr.timeout = 60000;
> io_hdr.pack_id = 0;
>
> The following change shrinks the size of sense_buffer to 128.
>
> Sebastian
>
> diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
> index 87e9a8d..91b3d90 100644
> --- a/libmultipath/prioritizers/emc.c
> +++ b/libmultipath/prioritizers/emc.c
> @@ -14,15 +14,15 @@
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> - unsigned char sense_buffer[256];
> + unsigned char sense_buffer[128];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> - sizeof(sb), 0};
> + sizeof(sense_buffer), 0};
> struct sg_io_hdr io_hdr;
> int ret = PRIO_UNDEF;
>
> memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
> - memset(&sense_buffer, 0, 256);
> + memset(&sense_buffer, 0, 128);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
Ping! Any comments?
Sebastian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len
2014-05-21 11:14 ` Sebastian Herbszt
@ 2014-05-21 22:46 ` Sebastian Herbszt
0 siblings, 0 replies; 5+ messages in thread
From: Sebastian Herbszt @ 2014-05-21 22:46 UTC (permalink / raw)
To: dm-devel, Christophe Varoqui; +Cc: Sebastian Herbszt
> Hi,
>
> have you verified this part is not enough to fix the problem, without
> shrinking the response size to 128 ?
>
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> - sizeof(sb), 0};
> + sizeof(sense_buffer), 0};
sizeof(sense_buffer) is 256 and it gets truncated to 0:
ioctl(4, SG_IO, {'S', SG_DXFER_FROM_DEV, cmd[6]=[12, 01, c0, 00, 00, 00], ...
[ 3730.100199] [5674]: scst: scst_parse_cmd:828:Warning: expected transfer length 256 for opcode INQUIRY (handler vdisk_blockio, target scst_local) doesn't match decoded value 0
[ 3730.100203] [5674]: scst_parse_cmd:830:Suspicious CDB:
[ 3730.100204] (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[ 3730.100206] 0: 12 01 c0 00 00 00 ......
This works
@@ -16,8 +16,9 @@ int emc_clariion_prio(const char *dev, int fd)
{
unsigned char sense_buffer[256];
unsigned char sb[128];
- unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
- sizeof(sb), 0};
+ unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0,
+ (sizeof(sense_buffer) >> 8) & 0xff,
+ sizeof(sense_buffer) & 0xff, 0};
struct sg_io_hdr io_hdr;
int ret = PRIO_UNDEF;
but I am not sure a buffer of 256 is required, because the checker just uses
128 bytes for the same page.
> Is it me or the variable name is just plain misleading ?
Should be "resp" or something like that.
> Best regards,
> Christophe Varoqui
> www.opensvc.com
Please keep me CC'ed in replies.
Sebastian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len
2014-04-05 11:13 multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len Sebastian Herbszt
2014-05-21 11:14 ` Sebastian Herbszt
@ 2014-05-21 20:31 ` Christophe Varoqui
2014-06-10 5:58 ` Christophe Varoqui
2 siblings, 0 replies; 5+ messages in thread
From: Christophe Varoqui @ 2014-05-21 20:31 UTC (permalink / raw)
To: device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 3015 bytes --]
Hi,
have you verified this part is not enough to fix the problem, without
shrinking the response size to 128 ?
unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
- sizeof(sb), 0};
+ sizeof(sense_buffer), 0};
Is it me or the variable name is just plain misleading ?
Best regards,
Christophe Varoqui
www.opensvc.com
On Sat, Apr 5, 2014 at 1:13 PM, Sebastian Herbszt <herbszt@gmx.de> wrote:
> Hello,
>
> while playing with multipath-tools and scst I got the following warning:
>
> [ 6382.467569] [3806]: scst: scst_parse_cmd:826:Warning: expected
> transfer length 256 for opcode 0x12 (handler vdisk_blockio, target
> scst_local) doesn't match decoded value 128
> [ 6382.467574] [3806]: scst_parse_cmd:828:Suspicious CDB:
> [ 6382.467579] (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
> [ 6382.467581] 0: 12 01 c0 00 80 00 ......
>
> This happened when my scst target pretended to be a CLARiiON.
> I noticed a mismatch in emc_clariion_prio (libmultipath/prioritizers/emc.c)
> between the allocation length and dxfer_len:
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> unsigned char sense_buffer[256];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> sizeof(sb), 0};
>
> ...
>
> memset(&sense_buffer, 0, 256);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
> io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
> io_hdr.dxfer_len = sizeof (sense_buffer);
> io_hdr.dxferp = sense_buffer;
> io_hdr.cmdp = inqCmdBlk;
> io_hdr.sbp = sb;
> io_hdr.timeout = 60000;
> io_hdr.pack_id = 0;
>
> The following change shrinks the size of sense_buffer to 128.
>
> Sebastian
>
> diff --git a/libmultipath/prioritizers/emc.c
> b/libmultipath/prioritizers/emc.c
> index 87e9a8d..91b3d90 100644
> --- a/libmultipath/prioritizers/emc.c
> +++ b/libmultipath/prioritizers/emc.c
> @@ -14,15 +14,15 @@
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> - unsigned char sense_buffer[256];
> + unsigned char sense_buffer[128];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> - sizeof(sb), 0};
> + sizeof(sense_buffer), 0};
> struct sg_io_hdr io_hdr;
> int ret = PRIO_UNDEF;
>
> memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
> - memset(&sense_buffer, 0, 256);
> + memset(&sense_buffer, 0, 128);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>
[-- Attachment #1.2: Type: text/html, Size: 4625 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len
2014-04-05 11:13 multipath-tools: mismatch between CDB allocation length and SG_IO dxfer_len Sebastian Herbszt
2014-05-21 11:14 ` Sebastian Herbszt
2014-05-21 20:31 ` Christophe Varoqui
@ 2014-06-10 5:58 ` Christophe Varoqui
2 siblings, 0 replies; 5+ messages in thread
From: Christophe Varoqui @ 2014-06-10 5:58 UTC (permalink / raw)
To: device-mapper development; +Cc: Sebastian Herbszt
[-- Attachment #1.1: Type: text/plain, Size: 2729 bytes --]
Applied.
I received the same patch for the rdac prioritizer
from netapp. Democratically, you're right.
Regards,
Christophe Varoqui
www.opensvc.com
On Sat, Apr 5, 2014 at 1:13 PM, Sebastian Herbszt <herbszt@gmx.de> wrote:
> Hello,
>
> while playing with multipath-tools and scst I got the following warning:
>
> [ 6382.467569] [3806]: scst: scst_parse_cmd:826:Warning: expected
> transfer length 256 for opcode 0x12 (handler vdisk_blockio, target
> scst_local) doesn't match decoded value 128
> [ 6382.467574] [3806]: scst_parse_cmd:828:Suspicious CDB:
> [ 6382.467579] (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
> [ 6382.467581] 0: 12 01 c0 00 80 00 ......
>
> This happened when my scst target pretended to be a CLARiiON.
> I noticed a mismatch in emc_clariion_prio (libmultipath/prioritizers/emc.c)
> between the allocation length and dxfer_len:
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> unsigned char sense_buffer[256];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> sizeof(sb), 0};
>
> ...
>
> memset(&sense_buffer, 0, 256);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
> io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
> io_hdr.dxfer_len = sizeof (sense_buffer);
> io_hdr.dxferp = sense_buffer;
> io_hdr.cmdp = inqCmdBlk;
> io_hdr.sbp = sb;
> io_hdr.timeout = 60000;
> io_hdr.pack_id = 0;
>
> The following change shrinks the size of sense_buffer to 128.
>
> Sebastian
>
> diff --git a/libmultipath/prioritizers/emc.c
> b/libmultipath/prioritizers/emc.c
> index 87e9a8d..91b3d90 100644
> --- a/libmultipath/prioritizers/emc.c
> +++ b/libmultipath/prioritizers/emc.c
> @@ -14,15 +14,15 @@
>
> int emc_clariion_prio(const char *dev, int fd)
> {
> - unsigned char sense_buffer[256];
> + unsigned char sense_buffer[128];
> unsigned char sb[128];
> unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
> - sizeof(sb), 0};
> + sizeof(sense_buffer), 0};
> struct sg_io_hdr io_hdr;
> int ret = PRIO_UNDEF;
>
> memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
> - memset(&sense_buffer, 0, 256);
> + memset(&sense_buffer, 0, 128);
> io_hdr.interface_id = 'S';
> io_hdr.cmd_len = sizeof (inqCmdBlk);
> io_hdr.mx_sb_len = sizeof (sb);
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>
[-- Attachment #1.2: Type: text/html, Size: 3691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread