* [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
@ 2022-01-12 22:06 Kees Cook
2022-01-12 22:23 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2022-01-12 22:06 UTC (permalink / raw)
To: Jens Axboe
Cc: Kees Cook, linux-ide, Damien Le Moal, linux-kernel, linux-hardening
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();
| ^~~~~~~~~~~~~~~~~~~~~~~~
Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-ide@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
improvements that are close to being finished. :)
---
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 3b31a4f596d8..c5a2c1e9ed6b 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)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
2022-01-12 22:06 [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region Kees Cook
@ 2022-01-12 22:23 ` Jens Axboe
2022-01-12 23:15 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2022-01-12 22:23 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-ide, Damien Le Moal, linux-kernel, linux-hardening
On 1/12/22 3:06 PM, 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();
> | ^~~~~~~~~~~~~~~~~~~~~~~~
>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: linux-ide@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
> improvements that are close to being finished. :)
I don't maintain libata anymore, so Damien is the guy to nudge ;-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
2022-01-12 22:23 ` Jens Axboe
@ 2022-01-12 23:15 ` Kees Cook
2022-01-12 23:47 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2022-01-12 23:15 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-ide, Damien Le Moal, linux-kernel, linux-hardening
On Wed, Jan 12, 2022 at 03:23:40PM -0700, Jens Axboe wrote:
> On 1/12/22 3:06 PM, 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();
> > | ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Cc: Jens Axboe <axboe@kernel.dk>
> > Cc: linux-ide@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
> > improvements that are close to being finished. :)
>
> I don't maintain libata anymore, so Damien is the guy to nudge ;-)
Ah-ha, okay, thanks.
/me waves "hi" to Damien. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
2022-01-12 23:15 ` Kees Cook
@ 2022-01-12 23:47 ` Damien Le Moal
2022-01-13 0:30 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2022-01-12 23:47 UTC (permalink / raw)
To: Kees Cook, Jens Axboe; +Cc: linux-ide, linux-kernel, linux-hardening
On 1/13/22 08:15, Kees Cook wrote:
> On Wed, Jan 12, 2022 at 03:23:40PM -0700, Jens Axboe wrote:
>> On 1/12/22 3:06 PM, 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();
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Cc: Jens Axboe <axboe@kernel.dk>
>>> Cc: linux-ide@vger.kernel.org
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
>>> improvements that are close to being finished. :)
>>
>> I don't maintain libata anymore, so Damien is the guy to nudge ;-)
>
> Ah-ha, okay, thanks.
>
> /me waves "hi" to Damien. :)
Hi Kees,
This is already queued up in libata tree for-5.17 branch. I have not
sent my PR to Linus yet as I am letting things soack a little longer in
for-next (for the various arch compile tests).
Please check that branch to see if all is OK !
Cheers.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
2022-01-12 23:47 ` Damien Le Moal
@ 2022-01-13 0:30 ` Kees Cook
2022-01-13 0:34 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2022-01-13 0:30 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Jens Axboe, linux-ide, linux-kernel, linux-hardening
On Thu, Jan 13, 2022 at 08:47:37AM +0900, Damien Le Moal wrote:
> On 1/13/22 08:15, Kees Cook wrote:
> > On Wed, Jan 12, 2022 at 03:23:40PM -0700, Jens Axboe wrote:
> >> On 1/12/22 3:06 PM, 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();
> >>> | ^~~~~~~~~~~~~~~~~~~~~~~~
> >>>
> >>> Cc: Jens Axboe <axboe@kernel.dk>
> >>> Cc: linux-ide@vger.kernel.org
> >>> Signed-off-by: Kees Cook <keescook@chromium.org>
> >>> ---
> >>> Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
> >>> improvements that are close to being finished. :)
> >>
> >> I don't maintain libata anymore, so Damien is the guy to nudge ;-)
> >
> > Ah-ha, okay, thanks.
> >
> > /me waves "hi" to Damien. :)
>
> Hi Kees,
>
> This is already queued up in libata tree for-5.17 branch. I have not
> sent my PR to Linus yet as I am letting things soack a little longer in
> for-next (for the various arch compile tests).
Oh thank you! Sorry I missed the pull. I didn't see it in -next yet, so
I assumed it hadn't been pulled anywhere.
> Please check that branch to see if all is OK !
Found it:
https://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata.git/log/?h=for-next
Yup, looks good:
https://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata.git/commit/?h=for-next&id=23c72ffedeed6d513144fa09834b1eb0cb2b7373
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region
2022-01-13 0:30 ` Kees Cook
@ 2022-01-13 0:34 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-01-13 0:34 UTC (permalink / raw)
To: Kees Cook; +Cc: Jens Axboe, linux-ide, linux-kernel, linux-hardening
On 1/13/22 09:30, Kees Cook wrote:
> On Thu, Jan 13, 2022 at 08:47:37AM +0900, Damien Le Moal wrote:
>> On 1/13/22 08:15, Kees Cook wrote:
>>> On Wed, Jan 12, 2022 at 03:23:40PM -0700, Jens Axboe wrote:
>>>> On 1/12/22 3:06 PM, 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();
>>>>> | ^~~~~~~~~~~~~~~~~~~~~~~~
>>>>>
>>>>> Cc: Jens Axboe <axboe@kernel.dk>
>>>>> Cc: linux-ide@vger.kernel.org
>>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>> ---
>>>>> Jens, can you take (or Ack) this? It's a dependency for the FORTIFY_SOURCE
>>>>> improvements that are close to being finished. :)
>>>>
>>>> I don't maintain libata anymore, so Damien is the guy to nudge ;-)
>>>
>>> Ah-ha, okay, thanks.
>>>
>>> /me waves "hi" to Damien. :)
>>
>> Hi Kees,
>>
>> This is already queued up in libata tree for-5.17 branch. I have not
>> sent my PR to Linus yet as I am letting things soack a little longer in
>> for-next (for the various arch compile tests).
>
> Oh thank you! Sorry I missed the pull. I didn't see it in -next yet, so
> I assumed it hadn't been pulled anywhere.
Uh... Weird. That one has been in libata for-next since a while back. So
it should be in linux-next.
>
>> Please check that branch to see if all is OK !
>
> Found it:
> https://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata.git/log/?h=for-next
>
> Yup, looks good:
> https://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata.git/commit/?h=for-next&id=23c72ffedeed6d513144fa09834b1eb0cb2b7373
OK !
>
> Thanks!
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-13 0:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 22:06 [PATCH][RESEND] sata_fsl: Use struct_group() for memcpy() region Kees Cook
2022-01-12 22:23 ` Jens Axboe
2022-01-12 23:15 ` Kees Cook
2022-01-12 23:47 ` Damien Le Moal
2022-01-13 0:30 ` Kees Cook
2022-01-13 0:34 ` Damien Le Moal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.