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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
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 [email protected].....
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 <[email protected]>
> > ---
> > 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
> >
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 <[email protected]>
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
>