lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Nov 2021 20:37:58 +0900 From: Damien Le Moal <damien.lemoal@...nsource.wdc.com> To: Kees Cook <keescook@...omium.org>, Jens Axboe <axboe@...nel.dk> Cc: linux-kernel@...r.kernel.org, linux-ide@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH] sata_fsl: Use struct_group() for memcpy() region On 2021/11/19 3:38, Kees Cook wrote: > In preparation for FORTIFY_SOURCE performing compile-time and run-time > field bounds checking for memcpy(), memmove(), and memset(), avoid > intentionally writing across neighboring fields. > > Use struct_group() in struct command_desc around members acmd and fill, > so they can be referenced together. This will allow memset(), memcpy(), > and sizeof() to more easily reason about sizes, improve readability, > and avoid future warnings about writing beyond the end of acmd: > > In function 'fortify_memset_chk', > inlined from 'sata_fsl_qc_prep' at drivers/ata/sata_fsl.c:534:3: > ./include/linux/fortify-string.h:199:4: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning] > 199 | __write_overflow_field(); > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Kees Cook <keescook@...omium.org> > --- > drivers/ata/sata_fsl.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c > index e5838b23c9e0..fec3c9032606 100644 > --- a/drivers/ata/sata_fsl.c > +++ b/drivers/ata/sata_fsl.c > @@ -246,8 +246,10 @@ enum { > struct command_desc { > u8 cfis[8 * 4]; > u8 sfis[8 * 4]; > - u8 acmd[4 * 4]; > - u8 fill[4 * 4]; > + struct_group(cdb, > + u8 acmd[4 * 4]; > + u8 fill[4 * 4]; > + ); > u32 prdt[SATA_FSL_MAX_PRD_DIRECT * 4]; > u32 prdt_indirect[(SATA_FSL_MAX_PRD - SATA_FSL_MAX_PRD_DIRECT) * 4]; > }; > @@ -531,8 +533,8 @@ static enum ata_completion_errors sata_fsl_qc_prep(struct ata_queued_cmd *qc) > /* setup "ACMD - atapi command" in cmd. desc. if this is ATAPI cmd */ > if (ata_is_atapi(qc->tf.protocol)) { > desc_info |= ATAPI_CMD; > - memset((void *)&cd->acmd, 0, 32); > - memcpy((void *)&cd->acmd, qc->cdb, qc->dev->cdb_len); > + memset(&cd->cdb, 0, sizeof(cd->cdb)); > + memcpy(&cd->cdb, qc->cdb, qc->dev->cdb_len); > } > > if (qc->flags & ATA_QCFLAG_DMAMAP) > Applied to for-5.17. Thanks. -- Damien Le Moal Western Digital Research
Powered by blists - more mailing lists