This series aims to replace one-element arrays with flexible-array
members in multiple structures in drivers/scsi/aacraid/aacraid.h.
There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].
This helps with the ongoing efforts to globally enable -Warray-bounds
and get us closer to being able to tighten the FORTIFY_SOURCE routines
on memcpy().
These issues were found with the help of Coccinelle and audited and fixed,
manually.
[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Gustavo A. R. Silva (8):
scsi: aacraid: Replace one-element array with flexible-array member
scsi: aacraid: Replace one-element array with flexible-array member in
struct sgmap
scsi: aacraid: Replace one-element array with flexible-array member in
struct user_sgmap
scsi: aacraid: Replace one-element array with flexible-array member in
struct sgmap64
scsi: aacraid: Replace one-element array with flexible-array member in
struct user_sgmap64
scsi: aacraid: Replace one-element array with flexible-array member in
struct sgmapraw
scsi: aacraid: Replace one-element array with flexible-array member in
struct user_sgmapraw
scsi: aacraid: Replace one-element array with flexible-array member in
struct aac_aifcmd
drivers/scsi/aacraid/aachba.c | 76 +++++++++++----------------------
drivers/scsi/aacraid/aacraid.h | 16 +++----
drivers/scsi/aacraid/commctrl.c | 5 +--
drivers/scsi/aacraid/comminit.c | 3 +-
4 files changed, 37 insertions(+), 63 deletions(-)
--
2.27.0
Replace one-element array with flexible-array member in struct
aac_ciss_phys_luns_resp.
Also, use the struct_size() helper to properly calculate the total size
for allocation.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 5 ++---
drivers/scsi/aacraid/aacraid.h | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index b04d039da276..98100e28e95e 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1823,13 +1823,12 @@ static inline void aac_free_safw_ciss_luns(struct aac_dev *dev)
static int aac_get_safw_ciss_luns(struct aac_dev *dev)
{
int rcode = -ENOMEM;
- int datasize;
+ size_t datasize;
struct aac_srb *srbcmd;
struct aac_srb_unit srbu;
struct aac_ciss_phys_luns_resp *phys_luns;
- datasize = sizeof(struct aac_ciss_phys_luns_resp) +
- (AAC_MAX_TARGETS - 1) * sizeof(struct _ciss_lun);
+ datasize = struct_size(phys_luns, lun, AAC_MAX_TARGETS);
phys_luns = kmalloc(datasize, GFP_KERNEL);
if (phys_luns == NULL)
goto out;
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 3733df77bc65..704440a96daa 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -321,7 +321,7 @@ struct aac_ciss_phys_luns_resp {
u8 level3[2];
u8 level2[2];
u8 node_ident[16]; /* phys. node identifier */
- } lun[1]; /* List of phys. devices */
+ } lun[]; /* List of phys. devices */
};
/*
--
2.27.0
Replace one-element array with flexible-array member in struct sgmap.
Also, make use of the struct_size() helper and refactor the code
according to the flexible array transformation.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 38 +++++++++++----------------------
drivers/scsi/aacraid/aacraid.h | 2 +-
drivers/scsi/aacraid/commctrl.c | 3 +--
drivers/scsi/aacraid/comminit.c | 3 +--
4 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 98100e28e95e..5014d8374916 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1316,7 +1316,7 @@ static int aac_read_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
static int aac_read_block(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count)
{
- u16 fibsize;
+ size_t fibsize;
struct aac_read *readcmd;
struct aac_dev *dev = fib->dev;
long ret;
@@ -1332,9 +1332,7 @@ static int aac_read_block(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32
ret = aac_build_sg(cmd, &readcmd->sg);
if (ret < 0)
return ret;
- fibsize = sizeof(struct aac_read) +
- ((le32_to_cpu(readcmd->sg.count) - 1) *
- sizeof (struct sgentry));
+ fibsize = struct_size(readcmd, sg.sg, le32_to_cpu(readcmd->sg.count));
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
/*
@@ -1450,7 +1448,7 @@ static int aac_write_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba,
static int aac_write_block(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count, int fua)
{
- u16 fibsize;
+ size_t fibsize;
struct aac_write *writecmd;
struct aac_dev *dev = fib->dev;
long ret;
@@ -1468,9 +1466,7 @@ static int aac_write_block(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
ret = aac_build_sg(cmd, &writecmd->sg);
if (ret < 0)
return ret;
- fibsize = sizeof(struct aac_write) +
- ((le32_to_cpu(writecmd->sg.count) - 1) *
- sizeof (struct sgentry));
+ fibsize = struct_size(writecmd, sg.sg, le32_to_cpu(writecmd->sg.count));
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
/*
@@ -1574,8 +1570,8 @@ static void aac_srb_callback(void *context, struct fib * fibptr);
static int aac_scsi_64(struct fib * fib, struct scsi_cmnd * cmd)
{
- u16 fibsize;
- struct aac_srb * srbcmd = aac_scsi_common(fib, cmd);
+ size_t fibsize;
+ struct aac_srb *srbcmd = aac_scsi_common(fib, cmd);
long ret;
ret = aac_build_sg64(cmd, (struct sgmap64 *) &srbcmd->sg);
@@ -1588,9 +1584,7 @@ static int aac_scsi_64(struct fib * fib, struct scsi_cmnd * cmd)
/*
* Build Scatter/Gather list
*/
- fibsize = sizeof (struct aac_srb) - sizeof (struct sgentry) +
- ((le32_to_cpu(srbcmd->sg.count) & 0xff) *
- sizeof (struct sgentry64));
+ fibsize = struct_size(srbcmd, sg.sg, le32_to_cpu(srbcmd->sg.count) & 0xff);
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
@@ -1605,8 +1599,8 @@ static int aac_scsi_64(struct fib * fib, struct scsi_cmnd * cmd)
static int aac_scsi_32(struct fib * fib, struct scsi_cmnd * cmd)
{
- u16 fibsize;
- struct aac_srb * srbcmd = aac_scsi_common(fib, cmd);
+ size_t fibsize;
+ struct aac_srb *srbcmd = aac_scsi_common(fib, cmd);
long ret;
ret = aac_build_sg(cmd, (struct sgmap *)&srbcmd->sg);
@@ -1619,9 +1613,7 @@ static int aac_scsi_32(struct fib * fib, struct scsi_cmnd * cmd)
/*
* Build Scatter/Gather list
*/
- fibsize = sizeof (struct aac_srb) +
- (((le32_to_cpu(srbcmd->sg.count) & 0xff) - 1) *
- sizeof (struct sgentry));
+ fibsize = struct_size(srbcmd, sg.sg, le32_to_cpu(srbcmd->sg.count) & 0xff);
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
@@ -1670,7 +1662,7 @@ static int aac_send_safw_bmic_cmd(struct aac_dev *dev,
struct fib *fibptr;
dma_addr_t addr;
int rcode;
- int fibsize;
+ size_t fibsize;
struct aac_srb *srb;
struct aac_srb_reply *srb_reply;
struct sgmap64 *sg64;
@@ -1689,8 +1681,7 @@ static int aac_send_safw_bmic_cmd(struct aac_dev *dev,
fibptr->hw_fib_va->header.XferState &=
~cpu_to_le32(FastResponseCapable);
- fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry) +
- sizeof(struct sgentry64);
+ fibsize = sizeof(struct aac_srb) + sizeof(struct sgentry64);
/* allocate DMA buffer for response */
addr = dma_map_single(&dev->pdev->dev, xfer_buf, xfer_len,
@@ -2261,8 +2252,7 @@ int aac_get_adapter_info(struct aac_dev* dev)
} else {
dev->a_ops.adapter_bounds = aac_bounds_32;
dev->scsi_host_ptr->sg_tablesize = (dev->max_fib_size -
- sizeof(struct aac_fibhdr) -
- sizeof(struct aac_write) + sizeof(struct sgentry)) /
+ sizeof(struct aac_fibhdr) - sizeof(struct aac_write)) /
sizeof(struct sgentry);
if (dev->dac_support) {
dev->a_ops.adapter_read = aac_read_block64;
@@ -3795,8 +3785,6 @@ static long aac_build_sg(struct scsi_cmnd *scsicmd, struct sgmap *psg)
// Get rid of old data
psg->count = 0;
- psg->sg[0].addr = 0;
- psg->sg[0].count = 0;
nseg = scsi_dma_map(scsicmd);
if (nseg <= 0)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 704440a96daa..320a30865d58 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -506,7 +506,7 @@ struct sge_ieee1212 {
struct sgmap {
__le32 count;
- struct sgentry sg[1];
+ struct sgentry sg[];
};
struct user_sgmap {
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index e7cc927ed952..5d6b0d9da0df 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -561,8 +561,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
rcode = -EINVAL;
goto cleanup;
}
- actual_fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry) +
- ((user_srbcmd->sg.count & 0xff) * sizeof(struct sgentry));
+ actual_fibsize = struct_size(srbcmd, sg.sg, user_srbcmd->sg.count & 0xff);
actual_fibsize64 = actual_fibsize + (user_srbcmd->sg.count & 0xff) *
(sizeof(struct sgentry64) - sizeof(struct sgentry));
/* User made a mistake - should not continue */
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 355b16f0b145..5f4a97f1f562 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -522,8 +522,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
spin_lock_init(&dev->iq_lock);
dev->max_fib_size = sizeof(struct hw_fib);
dev->sg_tablesize = host->sg_tablesize = (dev->max_fib_size
- - sizeof(struct aac_fibhdr)
- - sizeof(struct aac_write) + sizeof(struct sgentry))
+ - sizeof(struct aac_fibhdr) - sizeof(struct aac_write))
/ sizeof(struct sgentry);
dev->comm_interface = AAC_COMM_PRODUCER;
dev->raw_io_interface = dev->raw_io_64 = 0;
--
2.27.0
Replace one-element array with flexible-array member in struct
user_sgmap.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
drivers/scsi/aacraid/commctrl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 320a30865d58..bcf97c1b8c0c 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -511,7 +511,7 @@ struct sgmap {
struct user_sgmap {
u32 count;
- struct user_sgentry sg[1];
+ struct user_sgentry sg[];
};
struct sgmap64 {
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 5d6b0d9da0df..3206d9491fca 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -523,7 +523,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
goto cleanup;
}
- if ((fibsize < (sizeof(struct user_aac_srb) - sizeof(struct user_sgentry))) ||
+ if ((fibsize < sizeof(*user_srbcmd)) ||
(fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr)))) {
rcode = -EINVAL;
goto cleanup;
--
2.27.0
Replace one-element array with flexible-array member in struct sgmap64.
Also, make use of the struct_size() helper and refactor the code
according to the flexible array transformation in struct sgmap64.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 15 +++++----------
drivers/scsi/aacraid/aacraid.h | 2 +-
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 5014d8374916..02b75a2c3a88 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1281,7 +1281,7 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
static int aac_read_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count)
{
- u16 fibsize;
+ size_t fibsize;
struct aac_read64 *readcmd;
long ret;
@@ -1297,9 +1297,7 @@ static int aac_read_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
ret = aac_build_sg64(cmd, &readcmd->sg);
if (ret < 0)
return ret;
- fibsize = sizeof(struct aac_read64) +
- ((le32_to_cpu(readcmd->sg.count) - 1) *
- sizeof (struct sgentry64));
+ fibsize = struct_size(readcmd, sg.sg, le32_to_cpu(readcmd->sg.count));
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
/*
@@ -1413,7 +1411,7 @@ static int aac_write_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
static int aac_write_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count, int fua)
{
- u16 fibsize;
+ size_t fibsize;
struct aac_write64 *writecmd;
long ret;
@@ -1429,9 +1427,7 @@ static int aac_write_block64(struct fib * fib, struct scsi_cmnd * cmd, u64 lba,
ret = aac_build_sg64(cmd, &writecmd->sg);
if (ret < 0)
return ret;
- fibsize = sizeof(struct aac_write64) +
- ((le32_to_cpu(writecmd->sg.count) - 1) *
- sizeof (struct sgentry64));
+ fibsize = struct_size(writecmd, sg.sg, le32_to_cpu(writecmd->sg.count));
BUG_ON (fibsize > (fib->dev->max_fib_size -
sizeof(struct aac_fibhdr)));
/*
@@ -2263,8 +2259,7 @@ int aac_get_adapter_info(struct aac_dev* dev)
dev->scsi_host_ptr->sg_tablesize =
(dev->max_fib_size -
sizeof(struct aac_fibhdr) -
- sizeof(struct aac_write64) +
- sizeof(struct sgentry64)) /
+ sizeof(struct aac_write64)) /
sizeof(struct sgentry64);
} else {
dev->a_ops.adapter_read = aac_read_block;
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index bcf97c1b8c0c..573cb36c2e15 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -516,7 +516,7 @@ struct user_sgmap {
struct sgmap64 {
__le32 count;
- struct sgentry64 sg[1];
+ struct sgentry64 sg[];
};
struct user_sgmap64 {
--
2.27.0
Replace one-element array with flexible-array member in struct user_sgmap64.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 573cb36c2e15..fa5a93b4dc7b 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -521,7 +521,7 @@ struct sgmap64 {
struct user_sgmap64 {
u32 count;
- struct user_sgentry64 sg[1];
+ struct user_sgentry64 sg[];
};
struct sgmapraw {
--
2.27.0
Replace one-element array with flexible-array member in struct sgmapraw.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aachba.c | 18 ++++++------------
drivers/scsi/aacraid/aacraid.h | 2 +-
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 02b75a2c3a88..5c089993cf2a 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1224,7 +1224,8 @@ static void io_callback(void *context, struct fib * fibptr);
static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count)
{
struct aac_dev *dev = fib->dev;
- u16 fibsize, command;
+ size_t fibsize;
+ u16 command;
long ret;
aac_fib_init(fib);
@@ -1262,8 +1263,7 @@ static int aac_read_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u3
if (ret < 0)
return ret;
command = ContainerRawIo;
- fibsize = sizeof(struct aac_raw_io) +
- ((le32_to_cpu(readcmd->sg.count)-1) * sizeof(struct sgentryraw));
+ fibsize = struct_size(readcmd, sg.sg, le32_to_cpu(readcmd->sg.count));
}
BUG_ON(fibsize > (fib->dev->max_fib_size - sizeof(struct aac_fibhdr)));
@@ -1348,7 +1348,8 @@ static int aac_read_block(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32
static int aac_write_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u32 count, int fua)
{
struct aac_dev *dev = fib->dev;
- u16 fibsize, command;
+ size_t fibsize;
+ u16 command;
long ret;
aac_fib_init(fib);
@@ -1392,8 +1393,7 @@ static int aac_write_raw_io(struct fib * fib, struct scsi_cmnd * cmd, u64 lba, u
if (ret < 0)
return ret;
command = ContainerRawIo;
- fibsize = sizeof(struct aac_raw_io) +
- ((le32_to_cpu(writecmd->sg.count)-1) * sizeof (struct sgentryraw));
+ fibsize = struct_size(writecmd, sg.sg, le32_to_cpu(writecmd->sg.count));
}
BUG_ON(fibsize > (fib->dev->max_fib_size - sizeof(struct aac_fibhdr)));
@@ -3861,12 +3861,6 @@ static long aac_build_sgraw(struct scsi_cmnd *scsicmd, struct sgmapraw *psg)
// Get rid of old data
psg->count = 0;
- psg->sg[0].next = 0;
- psg->sg[0].prev = 0;
- psg->sg[0].addr[0] = 0;
- psg->sg[0].addr[1] = 0;
- psg->sg[0].count = 0;
- psg->sg[0].flags = 0;
nseg = scsi_dma_map(scsicmd);
if (nseg <= 0)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index fa5a93b4dc7b..511a58ec5c9d 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -526,7 +526,7 @@ struct user_sgmap64 {
struct sgmapraw {
__le32 count;
- struct sgentryraw sg[1];
+ struct sgentryraw sg[];
};
struct user_sgmapraw {
--
2.27.0
Replace one-element array with flexible-array member in struct
user_sgmapraw.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 511a58ec5c9d..97948cd5f13c 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -531,7 +531,7 @@ struct sgmapraw {
struct user_sgmapraw {
u32 count;
- struct user_sgentryraw sg[1];
+ struct user_sgentryraw sg[];
};
struct creation_info
--
2.27.0
Replace one-element array with flexible-array member in struct
aac_aifcmd.
This issue was found with the help of Coccinelle and audited and fixed,
manually.
Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/scsi/aacraid/aacraid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 97948cd5f13c..447feabf5360 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2616,7 +2616,7 @@ struct aac_hba_info {
struct aac_aifcmd {
__le32 command; /* Tell host what type of notify this is */
__le32 seqnum; /* To allow ordering of reports (if necessary) */
- u8 data[1]; /* Undefined length (from kernel viewpoint) */
+ u8 data[]; /* Undefined length (from kernel viewpoint) */
};
/**
--
2.27.0
Hi all,
Friendly ping: who can review or comment on this series, please?
Thanks
--
Gustavo
On Tue, Feb 22, 2022 at 01:28:12AM -0600, Gustavo A. R. Silva wrote:
> This series aims to replace one-element arrays with flexible-array
> members in multiple structures in drivers/scsi/aacraid/aacraid.h.
>
> There is a regular need in the kernel to provide a way to declare having
> a dynamically sized set of trailing elements in a structure. Kernel code
> should always use “flexible array members”[1] for these cases. The older
> style of one-element or zero-length arrays should no longer be used[2].
>
> This helps with the ongoing efforts to globally enable -Warray-bounds
> and get us closer to being able to tighten the FORTIFY_SOURCE routines
> on memcpy().
>
> These issues were found with the help of Coccinelle and audited and fixed,
> manually.
>
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
>
> Link: https://github.com/KSPP/linux/issues/79
>
> Gustavo A. R. Silva (8):
> scsi: aacraid: Replace one-element array with flexible-array member
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct sgmap
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct user_sgmap
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct sgmap64
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct user_sgmap64
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct sgmapraw
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct user_sgmapraw
> scsi: aacraid: Replace one-element array with flexible-array member in
> struct aac_aifcmd
>
> drivers/scsi/aacraid/aachba.c | 76 +++++++++++----------------------
> drivers/scsi/aacraid/aacraid.h | 16 +++----
> drivers/scsi/aacraid/commctrl.c | 5 +--
> drivers/scsi/aacraid/comminit.c | 3 +-
> 4 files changed, 37 insertions(+), 63 deletions(-)
>
> --
> 2.27.0
>
Gustavo,
> Friendly ping: who can review or comment on this series, please?
I'm afraid I don't have any hardware to test it on and the generated
output differs substantially from the original code.
--
Martin K. Petersen Oracle Linux Engineering
On Tue, Mar 15, 2022 at 12:02:13AM -0400, Martin K. Petersen wrote:
>
> Gustavo,
>
> > Friendly ping: who can review or comment on this series, please?
>
> I'm afraid I don't have any hardware to test it on and the generated
> output differs substantially from the original code.
Yeah; this series requires careful review from the people that
knows the code better.
It took me a day of work to go through all the places that needed
to be changed due to the flexible array transformation. However,
due to the kind of changes, it'd be great to have a second opinion
or at least someone that could take a look at the changes.
Thanks
--
Gustavo
On Tue, Feb 22, 2022 at 01:31:07AM -0600, Gustavo A. R. Silva wrote:
> Replace one-element array with flexible-array member in struct
> aac_aifcmd.
>
> This issue was found with the help of Coccinelle and audited and fixed,
> manually.
>
> Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
> Link: https://github.com/KSPP/linux/issues/79
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> drivers/scsi/aacraid/aacraid.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 97948cd5f13c..447feabf5360 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2616,7 +2616,7 @@ struct aac_hba_info {
> struct aac_aifcmd {
> __le32 command; /* Tell host what type of notify this is */
> __le32 seqnum; /* To allow ordering of reports (if necessary) */
> - u8 data[1]; /* Undefined length (from kernel viewpoint) */
> + u8 data[]; /* Undefined length (from kernel viewpoint) */
> };
>
> /**
> --
> 2.27.0
>
FWIW, this patch solves an -Warray-bounds warning that is seen under
-fstrict-flex-arrays=3 (coming soon[1]):
../drivers/scsi/aacraid/commsup.c:1166:17: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
(((__le32 *)aifcmd->data)[1] == cpu_to_le32(3));
^ ~
../drivers/scsi/aacraid/aacraid.h:2620:2: note: array 'data' declared here
u8 data[1]; /* Undefined length (from kernel viewpoint) */
^
../drivers/scsi/aacraid/commsup.c:1286:20: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
((((__le32 *)aifcmd->data)[3]
^ ~
../drivers/scsi/aacraid/aacraid.h:2620:2: note: array 'data' declared here
u8 data[1]; /* Undefined length (from kernel viewpoint) */
^
[1] new flag in GCC and Clang:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836
https://reviews.llvm.org/D126864
--
Kees Cook
On Mon, Mar 14, 2022 at 11:22:23PM -0500, Gustavo A. R. Silva wrote:
> On Tue, Mar 15, 2022 at 12:02:13AM -0400, Martin K. Petersen wrote:
> >
> > Gustavo,
> >
> > > Friendly ping: who can review or comment on this series, please?
> >
> > I'm afraid I don't have any hardware to test it on and the generated
> > output differs substantially from the original code.
>
> Yeah; this series requires careful review from the people that
> knows the code better.
>
> It took me a day of work to go through all the places that needed
> to be changed due to the flexible array transformation. However,
> due to the kind of changes, it'd be great to have a second opinion
> or at least someone that could take a look at the changes.
If the int/size_t changes are separated from the array size change, it's
easier to see the array size change is a binary no-op. (i.e. diffoscope
shows no executable changes.)
I'd recommend splitting the int/size_t changes from the array size
changes.
--
Kees Cook