* [PATCH] cxl/hdm: Enhance handling of invalid decoders
@ 2024-01-30 6:05 Quanquan Cao
2024-01-30 6:35 ` Dan Williams
0 siblings, 1 reply; 3+ messages in thread
From: Quanquan Cao @ 2024-01-30 6:05 UTC (permalink / raw
To: dave.jiang, vishal.l.verma; +Cc: linux-cxl, Quanquan Cao
Add the condition check "base + size < base", enhanced
handling of invalid decoders. This check ensures the
decoder's address range calculation won't overflow.
Signed-off-by: Quanquan Cao <caoqq@fujitsu.com>
---
drivers/cxl/core/hdm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 7d97790b893d..b8978d1c7a24 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -816,7 +816,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
if (!committed)
size = 0;
- if (base == U64_MAX || size == U64_MAX) {
+ if (base == U64_MAX || size == U64_MAX || base + size < base) {
dev_warn(&port->dev, "decoder%d.%d: Invalid resource range\n",
port->id, cxld->id);
return -ENXIO;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cxl/hdm: Enhance handling of invalid decoders
2024-01-30 6:05 [PATCH] cxl/hdm: Enhance handling of invalid decoders Quanquan Cao
@ 2024-01-30 6:35 ` Dan Williams
2024-01-30 6:49 ` Cao, Quanquan/曹 全全
0 siblings, 1 reply; 3+ messages in thread
From: Dan Williams @ 2024-01-30 6:35 UTC (permalink / raw
To: Quanquan Cao, dave.jiang, vishal.l.verma; +Cc: linux-cxl, Quanquan Cao
Quanquan Cao wrote:
> Add the condition check "base + size < base", enhanced
> handling of invalid decoders. This check ensures the
> decoder's address range calculation won't overflow.
>
> Signed-off-by: Quanquan Cao <caoqq@fujitsu.com>
> ---
> drivers/cxl/core/hdm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 7d97790b893d..b8978d1c7a24 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -816,7 +816,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>
> if (!committed)
> size = 0;
> - if (base == U64_MAX || size == U64_MAX) {
> + if (base == U64_MAX || size == U64_MAX || base + size < base) {
The U64_MAX checks were added for a device that returned all
0xffffffffffffffff on read. That happens to match the common expectation
that a device or a bus in an error state starts returning that value to
MMIO cycles. So this was less about validating that those values were
sane and more about having a convenient point to detect that the device
is returning the common indicator for MMIO failure.
Otherwise a device that reports an overflow condition is definitely
broken, but it will be caught by other address validation code. So, I
do not think this is suitable otherwise it would open the door for many
other device validation checks.
I would change my mind if there was an example of this breakage in the
wild causing real end user pain that is too late for the device hardware
vendor to fix.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cxl/hdm: Enhance handling of invalid decoders
2024-01-30 6:35 ` Dan Williams
@ 2024-01-30 6:49 ` Cao, Quanquan/曹 全全
0 siblings, 0 replies; 3+ messages in thread
From: Cao, Quanquan/曹 全全 @ 2024-01-30 6:49 UTC (permalink / raw
To: Dan Williams, dave.jiang, vishal.l.verma; +Cc: linux-cxl
> Quanquan Cao wrote:
>> Add the condition check "base + size < base", enhanced
>> handling of invalid decoders. This check ensures the
>> decoder's address range calculation won't overflow.
>>
>> Signed-off-by: Quanquan Cao <caoqq@fujitsu.com>
>> ---
>> drivers/cxl/core/hdm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index 7d97790b893d..b8978d1c7a24 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -816,7 +816,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>>
>> if (!committed)
>> size = 0;
>> - if (base == U64_MAX || size == U64_MAX) {
>> + if (base == U64_MAX || size == U64_MAX || base + size < base) {
>
> The U64_MAX checks were added for a device that returned all
> 0xffffffffffffffff on read. That happens to match the common expectation
> that a device or a bus in an error state starts returning that value to
> MMIO cycles. So this was less about validating that those values were
> sane and more about having a convenient point to detect that the device
> is returning the common indicator for MMIO failure.
>
> Otherwise a device that reports an overflow condition is definitely
> broken, but it will be caught by other address validation code. So, I
> do not think this is suitable otherwise it would open the door for many
> other device validation checks.
>
> I would change my mind if there was an example of this breakage in the
> wild causing real end user pain that is too late for the device hardware
> vendor to fix.
>
Thanks, I got.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-30 6:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-30 6:05 [PATCH] cxl/hdm: Enhance handling of invalid decoders Quanquan Cao
2024-01-30 6:35 ` Dan Williams
2024-01-30 6:49 ` Cao, Quanquan/曹 全全
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 read-only IMAP folder(s) and NNTP newsgroup(s).