linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two
@ 2019-12-21  7:09 Lubomir Rintel
  2019-12-30 17:03 ` Konrad Rzeszutek Wilk
  2020-01-12 18:22 ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 4+ messages in thread
From: Lubomir Rintel @ 2019-12-21  7:09 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Peter Jones, linux-kernel, Lubomir Rintel

According to iSCSI Boot Firmware Table Version 1.03 [1], the length of
the control table is ">= 18", where the optional expansion structure
pointer follow the mandatory ones. This allows for more than two NICs
and Targets.

[1] ftp://ftp.software.ibm.com/systems/support/bladecenter/iscsi_boot_firmware_table_v1.03.pdf

Let's enforce the minimum length of the control structure instead
instead of limiting it to the smallest allowed size.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/firmware/iscsi_ibft.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
index 7e12cbdf957cc..96758b71a8db8 100644
--- a/drivers/firmware/iscsi_ibft.c
+++ b/drivers/firmware/iscsi_ibft.c
@@ -104,6 +104,7 @@ struct ibft_control {
 	u16 tgt0_off;
 	u16 nic1_off;
 	u16 tgt1_off;
+	u16 expansion[0];
 } __attribute__((__packed__));
 
 struct ibft_initiator {
@@ -235,7 +236,7 @@ static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
 				"found %d instead!\n", t, id, hdr->id);
 		return -ENODEV;
 	}
-	if (hdr->length != length) {
+	if (length && hdr->length != length) {
 		printk(KERN_ERR "iBFT error: We expected the %s " \
 				"field header.length to have %d but " \
 				"found %d instead!\n", t, length, hdr->length);
@@ -749,16 +750,16 @@ static int __init ibft_register_kobjects(struct acpi_table_ibft *header)
 	control = (void *)header + sizeof(*header);
 	end = (void *)control + control->hdr.length;
 	eot_offset = (void *)header + header->header.length - (void *)control;
-	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
-			     sizeof(*control));
+	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control, 0);
 
 	/* iBFT table safety checking */
 	rc |= ((control->hdr.index) ? -ENODEV : 0);
+	rc |= ((control->hdr.length < sizeof(*control)) ? -ENODEV : 0);
 	if (rc) {
 		printk(KERN_ERR "iBFT error: Control header is invalid!\n");
 		return rc;
 	}
-	for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
+	for (ptr = &control->initiator_off; ptr + sizeof(u16) <= end; ptr += sizeof(u16)) {
 		offset = *(u16 *)ptr;
 		if (offset && offset < header->header.length &&
 						offset < eot_offset) {
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two
  2019-12-21  7:09 [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two Lubomir Rintel
@ 2019-12-30 17:03 ` Konrad Rzeszutek Wilk
  2020-01-11  9:38   ` Lubomir Rintel
  2020-01-12 18:22 ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2019-12-30 17:03 UTC (permalink / raw)
  To: Lubomir Rintel; +Cc: Konrad Rzeszutek Wilk, Peter Jones, linux-kernel

On Sat, Dec 21, 2019 at 08:09:56AM +0100, Lubomir Rintel wrote:
> According to iSCSI Boot Firmware Table Version 1.03 [1], the length of
> the control table is ">= 18", where the optional expansion structure
> pointer follow the mandatory ones. This allows for more than two NICs
> and Targets.
> 
> [1] ftp://ftp.software.ibm.com/systems/support/bladecenter/iscsi_boot_firmware_table_v1.03.pdf
> 
> Let's enforce the minimum length of the control structure instead
> instead of limiting it to the smallest allowed size.

Hi!

Do you have an example of such iBFT table? Thanks
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  drivers/firmware/iscsi_ibft.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
> index 7e12cbdf957cc..96758b71a8db8 100644
> --- a/drivers/firmware/iscsi_ibft.c
> +++ b/drivers/firmware/iscsi_ibft.c
> @@ -104,6 +104,7 @@ struct ibft_control {
>  	u16 tgt0_off;
>  	u16 nic1_off;
>  	u16 tgt1_off;
> +	u16 expansion[0];
>  } __attribute__((__packed__));
>  
>  struct ibft_initiator {
> @@ -235,7 +236,7 @@ static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
>  				"found %d instead!\n", t, id, hdr->id);
>  		return -ENODEV;
>  	}
> -	if (hdr->length != length) {
> +	if (length && hdr->length != length) {
>  		printk(KERN_ERR "iBFT error: We expected the %s " \
>  				"field header.length to have %d but " \
>  				"found %d instead!\n", t, length, hdr->length);
> @@ -749,16 +750,16 @@ static int __init ibft_register_kobjects(struct acpi_table_ibft *header)
>  	control = (void *)header + sizeof(*header);
>  	end = (void *)control + control->hdr.length;
>  	eot_offset = (void *)header + header->header.length - (void *)control;
> -	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
> -			     sizeof(*control));
> +	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control, 0);
>  
>  	/* iBFT table safety checking */
>  	rc |= ((control->hdr.index) ? -ENODEV : 0);
> +	rc |= ((control->hdr.length < sizeof(*control)) ? -ENODEV : 0);
>  	if (rc) {
>  		printk(KERN_ERR "iBFT error: Control header is invalid!\n");
>  		return rc;
>  	}
> -	for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
> +	for (ptr = &control->initiator_off; ptr + sizeof(u16) <= end; ptr += sizeof(u16)) {
>  		offset = *(u16 *)ptr;
>  		if (offset && offset < header->header.length &&
>  						offset < eot_offset) {
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two
  2019-12-30 17:03 ` Konrad Rzeszutek Wilk
@ 2020-01-11  9:38   ` Lubomir Rintel
  0 siblings, 0 replies; 4+ messages in thread
From: Lubomir Rintel @ 2020-01-11  9:38 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Konrad Rzeszutek Wilk, Peter Jones, linux-kernel

On Mon, 2019-12-30 at 12:03 -0500, Konrad Rzeszutek Wilk wrote:
> On Sat, Dec 21, 2019 at 08:09:56AM +0100, Lubomir Rintel wrote:
> > According to iSCSI Boot Firmware Table Version 1.03 [1], the length of
> > the control table is ">= 18", where the optional expansion structure
> > pointer follow the mandatory ones. This allows for more than two NICs
> > and Targets.
> > 
> > [1] ftp://ftp.software.ibm.com/systems/support/bladecenter/iscsi_boot_firmware_table_v1.03.pdf
> > 
> > Let's enforce the minimum length of the control structure instead
> > instead of limiting it to the smallest allowed size.
> 
> Hi!
> 
> Do you have an example of such iBFT table? Thanks

Sorry for the late response, your message somehow slipped throught the
cracks during the holidays.

Please feed the following to "xxd -r". The md5sum is
991d671cc36367da6b104cce120a6048. You can test it with
"qemu -acpitable file=ibft.img".

00000000: 69 42 46 54 fb 02 00 00 01 d2 54 45 53 54 49 4e  iBFT......TESTIN
00000010: 54 45 53 54 00 00 00 00 00 00 00 00 00 00 00 00  TEST............
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 01 01 16 00 00 00 00 00 48 00 98 00 d0 01 00 01  ........H.......
00000040: 08 02 68 01 40 02 00 00 02 01 4a 00 00 03 00 00  ..h.@.....J.....
00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 22 00  ..............".
00000090: 78 02 00 00 00 00 00 00 03 01 66 00 00 03 00 00  x.........f.....
000000a0: 00 00 00 00 00 00 00 00 ff ff c0 a8 32 65 18 03  ............2e..
000000b0: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 32 01  ..............2.
000000c0: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 32 01  ..............2.
000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
000000e0: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 32 01  ..............2.
000000f0: 00 00 52 54 00 12 34 00 18 00 06 00 9b 02 00 00  ..RT..4.........
00000100: 03 01 66 00 01 03 00 00 00 00 00 00 00 00 00 00  ..f.............
00000110: ff ff c0 a8 33 65 18 03 00 00 00 00 00 00 00 00  ....3e..........
00000120: 00 00 ff ff c0 a8 33 01 00 00 00 00 00 00 00 00  ......3.........
00000130: 00 00 ff ff c0 a8 33 01 00 00 00 00 00 00 00 00  ......3.........
00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000150: 00 00 ff ff c0 a8 33 01 00 00 52 54 00 12 34 01  ......3...RT..4.
00000160: 20 00 06 00 a2 02 00 00 03 01 66 00 02 03 00 00   .........f.....
00000170: 00 00 00 00 00 00 00 00 ff ff c0 a8 34 65 18 03  ............4e..
00000180: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 34 01  ..............4.
00000190: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 34 01  ..............4.
000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
000001b0: 00 00 00 00 00 00 00 00 00 00 ff ff c0 a8 34 01  ..............4.
000001c0: 00 00 52 54 00 12 34 01 28 00 06 00 a9 02 00 00  ..RT..4.(.......
000001d0: 04 01 36 00 00 03 00 00 00 00 00 00 00 00 00 00  ..6.............
000001e0: ff ff c0 a8 32 01 bc 0c 00 01 00 00 00 00 00 00  ....2...........
000001f0: 00 00 18 00 b0 02 00 00 00 00 00 00 00 00 00 00  ................
00000200: 00 00 00 00 00 00 00 00 04 01 36 00 01 03 00 00  ..........6.....
00000210: 00 00 00 00 00 00 00 00 ff ff c0 a8 33 01 bc 0c  ............3...
00000220: 00 02 00 00 00 00 00 00 00 00 18 00 c9 02 00 00  ................
00000230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000240: 04 01 36 00 02 03 00 00 00 00 00 00 00 00 00 00  ..6.............
00000250: ff ff c0 a8 34 01 bc 0c 00 03 00 00 00 00 00 00  ....4...........
00000260: 00 00 18 00 e2 02 00 00 00 00 00 00 00 00 00 00  ................
00000270: 00 00 00 00 00 00 00 00 69 71 6e 2e 31 39 39 34  ........iqn.1994
00000280: 2d 30 35 2e 63 6f 6d 2e 72 65 64 68 61 74 3a 36  -05.com.redhat:6
00000290: 33 33 31 31 34 61 61 63 66 32 00 63 6c 69 65 6e  33114aacf2.clien
000002a0: 74 00 63 6c 69 65 6e 74 00 63 6c 69 65 6e 74 00  t.client.client.
000002b0: 69 71 6e 2e 32 30 30 39 2d 30 36 2e 74 65 73 74  iqn.2009-06.test
000002c0: 3a 74 61 72 67 65 74 30 00 69 71 6e 2e 32 30 30  :target0.iqn.200
000002d0: 39 2d 30 36 2e 74 65 73 74 3a 74 61 72 67 65 74  9-06.test:target
000002e0: 31 00 69 71 6e 2e 32 30 30 39 2d 30 36 2e 74 65  1.iqn.2009-06.te
000002f0: 73 74 3a 74 61 72 67 65 74 32 00                 st:target2.

Thanks
Lubo

> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> >  drivers/firmware/iscsi_ibft.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
> > index 7e12cbdf957cc..96758b71a8db8 100644
> > --- a/drivers/firmware/iscsi_ibft.c
> > +++ b/drivers/firmware/iscsi_ibft.c
> > @@ -104,6 +104,7 @@ struct ibft_control {
> >  	u16 tgt0_off;
> >  	u16 nic1_off;
> >  	u16 tgt1_off;
> > +	u16 expansion[0];
> >  } __attribute__((__packed__));
> >  
> >  struct ibft_initiator {
> > @@ -235,7 +236,7 @@ static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
> >  				"found %d instead!\n", t, id, hdr->id);
> >  		return -ENODEV;
> >  	}
> > -	if (hdr->length != length) {
> > +	if (length && hdr->length != length) {
> >  		printk(KERN_ERR "iBFT error: We expected the %s " \
> >  				"field header.length to have %d but " \
> >  				"found %d instead!\n", t, length, hdr->length);
> > @@ -749,16 +750,16 @@ static int __init ibft_register_kobjects(struct acpi_table_ibft *header)
> >  	control = (void *)header + sizeof(*header);
> >  	end = (void *)control + control->hdr.length;
> >  	eot_offset = (void *)header + header->header.length - (void *)control;
> > -	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
> > -			     sizeof(*control));
> > +	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control, 0);
> >  
> >  	/* iBFT table safety checking */
> >  	rc |= ((control->hdr.index) ? -ENODEV : 0);
> > +	rc |= ((control->hdr.length < sizeof(*control)) ? -ENODEV : 0);
> >  	if (rc) {
> >  		printk(KERN_ERR "iBFT error: Control header is invalid!\n");
> >  		return rc;
> >  	}
> > -	for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
> > +	for (ptr = &control->initiator_off; ptr + sizeof(u16) <= end; ptr += sizeof(u16)) {
> >  		offset = *(u16 *)ptr;
> >  		if (offset && offset < header->header.length &&
> >  						offset < eot_offset) {
> > -- 
> > 2.24.1
> > 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two
  2019-12-21  7:09 [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two Lubomir Rintel
  2019-12-30 17:03 ` Konrad Rzeszutek Wilk
@ 2020-01-12 18:22 ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2020-01-12 18:22 UTC (permalink / raw)
  To: Lubomir Rintel; +Cc: Konrad Rzeszutek Wilk, Peter Jones, linux-kernel

On Sat, Dec 21, 2019 at 08:09:56AM +0100, Lubomir Rintel wrote:
> According to iSCSI Boot Firmware Table Version 1.03 [1], the length of
> the control table is ">= 18", where the optional expansion structure
> pointer follow the mandatory ones. This allows for more than two NICs
> and Targets.
> 
> [1] ftp://ftp.software.ibm.com/systems/support/bladecenter/iscsi_boot_firmware_table_v1.03.pdf
> 
> Let's enforce the minimum length of the control structure instead
> instead of limiting it to the smallest allowed size.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Put it in my tree and will send it up to Linus for the next merge
window.

Thanks!
> ---
>  drivers/firmware/iscsi_ibft.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
> index 7e12cbdf957cc..96758b71a8db8 100644
> --- a/drivers/firmware/iscsi_ibft.c
> +++ b/drivers/firmware/iscsi_ibft.c
> @@ -104,6 +104,7 @@ struct ibft_control {
>  	u16 tgt0_off;
>  	u16 nic1_off;
>  	u16 tgt1_off;
> +	u16 expansion[0];
>  } __attribute__((__packed__));
>  
>  struct ibft_initiator {
> @@ -235,7 +236,7 @@ static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
>  				"found %d instead!\n", t, id, hdr->id);
>  		return -ENODEV;
>  	}
> -	if (hdr->length != length) {
> +	if (length && hdr->length != length) {
>  		printk(KERN_ERR "iBFT error: We expected the %s " \
>  				"field header.length to have %d but " \
>  				"found %d instead!\n", t, length, hdr->length);
> @@ -749,16 +750,16 @@ static int __init ibft_register_kobjects(struct acpi_table_ibft *header)
>  	control = (void *)header + sizeof(*header);
>  	end = (void *)control + control->hdr.length;
>  	eot_offset = (void *)header + header->header.length - (void *)control;
> -	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
> -			     sizeof(*control));
> +	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control, 0);
>  
>  	/* iBFT table safety checking */
>  	rc |= ((control->hdr.index) ? -ENODEV : 0);
> +	rc |= ((control->hdr.length < sizeof(*control)) ? -ENODEV : 0);
>  	if (rc) {
>  		printk(KERN_ERR "iBFT error: Control header is invalid!\n");
>  		return rc;
>  	}
> -	for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
> +	for (ptr = &control->initiator_off; ptr + sizeof(u16) <= end; ptr += sizeof(u16)) {
>  		offset = *(u16 *)ptr;
>  		if (offset && offset < header->header.length &&
>  						offset < eot_offset) {
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-12 18:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-21  7:09 [RESEND PATCH] iscsi_ibft: Don't limits Targets and NICs to two Lubomir Rintel
2019-12-30 17:03 ` Konrad Rzeszutek Wilk
2020-01-11  9:38   ` Lubomir Rintel
2020-01-12 18:22 ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).