* [PATCH 1/1] tools/libs/light: fix BAR memory address truncation
@ 2025-10-10 8:00 Jiqian Chen
2025-10-10 8:19 ` Jan Beulich
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jiqian Chen @ 2025-10-10 8:00 UTC (permalink / raw)
To: xen-devel
Cc: Huang Rui, Jiqian Chen, Anthony PERARD, Juergen Gross,
Oleksii Kurochko
64-bit BAR memory address is truncated when removing a passthrough pci
device from guest since it uses "unsigned int".
So, change to use "unsigned long long" to fix this problem.
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
cc: Anthony PERARD <anthony.perard@vates.tech>
cc: Juergen Gross <jgross@suse.com>
cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
tools/libs/light/libxl_pci.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 2ea2caeb6624..88ffbbf08abe 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -2001,7 +2001,8 @@ static void pci_remove_detached(libxl__egc *egc,
{
STATE_AO_GC(prs->aodev->ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
- unsigned int start = 0, end = 0, flags = 0, size = 0, irq = 0;
+ unsigned long long start = 0, end = 0, flags = 0, size = 0;
+ unsigned int irq = 0;
int i, stubdomid = 0;
const char *sysfs_path;
FILE *f;
@@ -2031,7 +2032,7 @@ static void pci_remove_detached(libxl__egc *egc,
}
for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
- if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3)
+ if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
continue;
size = end - start + 1;
if (start) {
@@ -2040,7 +2041,7 @@ static void pci_remove_detached(libxl__egc *egc,
size, 0);
if (rc < 0)
LOGED(ERROR, domid,
- "xc_domain_ioport_permission error 0x%x/0x%x",
+ "xc_domain_ioport_permission error 0x%llx/0x%llx",
start,
size);
} else {
@@ -2050,7 +2051,7 @@ static void pci_remove_detached(libxl__egc *egc,
0);
if (rc < 0)
LOGED(ERROR, domid,
- "xc_domain_iomem_permission error 0x%x/0x%x",
+ "xc_domain_iomem_permission error 0x%llx/0x%llx",
start,
size);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] tools/libs/light: fix BAR memory address truncation
2025-10-10 8:00 [PATCH 1/1] tools/libs/light: fix BAR memory address truncation Jiqian Chen
@ 2025-10-10 8:19 ` Jan Beulich
2025-10-10 9:02 ` Jürgen Groß
2025-10-10 8:21 ` Jan Beulich
2025-10-13 7:40 ` Oleksii Kurochko
2 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2025-10-10 8:19 UTC (permalink / raw)
To: Jiqian Chen
Cc: Huang Rui, Anthony PERARD, Juergen Gross, Oleksii Kurochko,
xen-devel
On 10.10.2025 10:00, Jiqian Chen wrote:
> 64-bit BAR memory address is truncated when removing a passthrough pci
> device from guest since it uses "unsigned int".
You talking of address truncation only here, ...
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -2001,7 +2001,8 @@ static void pci_remove_detached(libxl__egc *egc,
> {
> STATE_AO_GC(prs->aodev->ao);
> libxl_ctx *ctx = libxl__gc_owner(gc);
> - unsigned int start = 0, end = 0, flags = 0, size = 0, irq = 0;
> + unsigned long long start = 0, end = 0, flags = 0, size = 0;
> + unsigned int irq = 0;
... does "flags" really need widening, too?
> @@ -2031,7 +2032,7 @@ static void pci_remove_detached(libxl__egc *egc,
> }
>
> for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
> - if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3)
> + if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
While touching this, don't we want to drop the stray 0x in here? Their
presence causes bogus input like 0x0x0 to be accepted, afaict.
> @@ -2040,7 +2041,7 @@ static void pci_remove_detached(libxl__egc *egc,
> size, 0);
> if (rc < 0)
> LOGED(ERROR, domid,
> - "xc_domain_ioport_permission error 0x%x/0x%x",
> + "xc_domain_ioport_permission error 0x%llx/0x%llx",
> start,
> size);
> } else {
> @@ -2050,7 +2051,7 @@ static void pci_remove_detached(libxl__egc *egc,
> 0);
> if (rc < 0)
> LOGED(ERROR, domid,
> - "xc_domain_iomem_permission error 0x%x/0x%x",
> + "xc_domain_iomem_permission error 0x%llx/0x%llx",
In the hypervisor I would request use of %#llx here; not sure what the
toolstack's take on this is.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] tools/libs/light: fix BAR memory address truncation
2025-10-10 8:00 [PATCH 1/1] tools/libs/light: fix BAR memory address truncation Jiqian Chen
2025-10-10 8:19 ` Jan Beulich
@ 2025-10-10 8:21 ` Jan Beulich
2025-10-13 7:40 ` Oleksii Kurochko
2 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2025-10-10 8:21 UTC (permalink / raw)
To: Jiqian Chen
Cc: Huang Rui, Anthony PERARD, Juergen Gross, Oleksii Kurochko,
xen-devel
On 10.10.2025 10:00, Jiqian Chen wrote:
> 64-bit BAR memory address is truncated when removing a passthrough pci
> device from guest since it uses "unsigned int".
>
> So, change to use "unsigned long long" to fix this problem.
>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Oh, and - please add a Fixes: tag when addressing bugs.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] tools/libs/light: fix BAR memory address truncation
2025-10-10 8:19 ` Jan Beulich
@ 2025-10-10 9:02 ` Jürgen Groß
0 siblings, 0 replies; 5+ messages in thread
From: Jürgen Groß @ 2025-10-10 9:02 UTC (permalink / raw)
To: Jan Beulich, Jiqian Chen
Cc: Huang Rui, Anthony PERARD, Oleksii Kurochko, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2901 bytes --]
On 10.10.25 10:19, Jan Beulich wrote:
> On 10.10.2025 10:00, Jiqian Chen wrote:
>> 64-bit BAR memory address is truncated when removing a passthrough pci
>> device from guest since it uses "unsigned int".
>
> You talking of address truncation only here, ...
>
>> --- a/tools/libs/light/libxl_pci.c
>> +++ b/tools/libs/light/libxl_pci.c
>> @@ -2001,7 +2001,8 @@ static void pci_remove_detached(libxl__egc *egc,
>> {
>> STATE_AO_GC(prs->aodev->ao);
>> libxl_ctx *ctx = libxl__gc_owner(gc);
>> - unsigned int start = 0, end = 0, flags = 0, size = 0, irq = 0;
>> + unsigned long long start = 0, end = 0, flags = 0, size = 0;
>> + unsigned int irq = 0;
>
> ... does "flags" really need widening, too?
At least on the system I looked the value was printed as a 64-bit one:
# cat /sys/bus/pci/devices/0000:00:00.0/resource
0x0000000000000000 0x0000000000000000 0x0000000000000000
...
So not widening flags would rely on UB to preserve the evaluated PCI_BAR_IO
flag in case the high 32 bits don't contain 0.
>
>> @@ -2031,7 +2032,7 @@ static void pci_remove_detached(libxl__egc *egc,
>> }
>>
>> for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
>> - if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3)
>> + if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
>
> While touching this, don't we want to drop the stray 0x in here? Their
> presence causes bogus input like 0x0x0 to be accepted, afaict.
Hmm, do we really expect a sysfs file to produce bogus output?
Wouldn't it be better to keep the "0x" in order to detect a differing
output format, which could result in silent misbehavior?
I'm not really feeling strong here, as both cases seem highly unlikely.
>
>> @@ -2040,7 +2041,7 @@ static void pci_remove_detached(libxl__egc *egc,
>> size, 0);
>> if (rc < 0)
>> LOGED(ERROR, domid,
>> - "xc_domain_ioport_permission error 0x%x/0x%x",
>> + "xc_domain_ioport_permission error 0x%llx/0x%llx",
>> start,
>> size);
>> } else {
>> @@ -2050,7 +2051,7 @@ static void pci_remove_detached(libxl__egc *egc,
>> 0);
>> if (rc < 0)
>> LOGED(ERROR, domid,
>> - "xc_domain_iomem_permission error 0x%x/0x%x",
>> + "xc_domain_iomem_permission error 0x%llx/0x%llx",
>
> In the hypervisor I would request use of %#llx here; not sure what the
> toolstack's take on this is.
I'd go a little bit further and request to use uint64_t instead of
"unsigned long long" and then use "#"PRIx64 for the format.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] tools/libs/light: fix BAR memory address truncation
2025-10-10 8:00 [PATCH 1/1] tools/libs/light: fix BAR memory address truncation Jiqian Chen
2025-10-10 8:19 ` Jan Beulich
2025-10-10 8:21 ` Jan Beulich
@ 2025-10-13 7:40 ` Oleksii Kurochko
2 siblings, 0 replies; 5+ messages in thread
From: Oleksii Kurochko @ 2025-10-13 7:40 UTC (permalink / raw)
To: Jiqian Chen, xen-devel; +Cc: Huang Rui, Anthony PERARD, Juergen Gross
[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]
On 10/10/25 10:00 AM, Jiqian Chen wrote:
> 64-bit BAR memory address is truncated when removing a passthrough pci
> device from guest since it uses "unsigned int".
>
> So, change to use "unsigned long long" to fix this problem.
>
> Signed-off-by: Jiqian Chen<Jiqian.Chen@amd.com>
> ---
> cc: Anthony PERARD<anthony.perard@vates.tech>
> cc: Juergen Gross<jgross@suse.com>
> cc: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> ---
> tools/libs/light/libxl_pci.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
With getting proper Ack(s) from maintainers, the patch could be considered
to be in 4.21:
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
>
> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index 2ea2caeb6624..88ffbbf08abe 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -2001,7 +2001,8 @@ static void pci_remove_detached(libxl__egc *egc,
> {
> STATE_AO_GC(prs->aodev->ao);
> libxl_ctx *ctx = libxl__gc_owner(gc);
> - unsigned int start = 0, end = 0, flags = 0, size = 0, irq = 0;
> + unsigned long long start = 0, end = 0, flags = 0, size = 0;
> + unsigned int irq = 0;
> int i, stubdomid = 0;
> const char *sysfs_path;
> FILE *f;
> @@ -2031,7 +2032,7 @@ static void pci_remove_detached(libxl__egc *egc,
> }
>
> for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
> - if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3)
> + if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
> continue;
> size = end - start + 1;
> if (start) {
> @@ -2040,7 +2041,7 @@ static void pci_remove_detached(libxl__egc *egc,
> size, 0);
> if (rc < 0)
> LOGED(ERROR, domid,
> - "xc_domain_ioport_permission error 0x%x/0x%x",
> + "xc_domain_ioport_permission error 0x%llx/0x%llx",
> start,
> size);
> } else {
> @@ -2050,7 +2051,7 @@ static void pci_remove_detached(libxl__egc *egc,
> 0);
> if (rc < 0)
> LOGED(ERROR, domid,
> - "xc_domain_iomem_permission error 0x%x/0x%x",
> + "xc_domain_iomem_permission error 0x%llx/0x%llx",
> start,
> size);
> }
[-- Attachment #2: Type: text/html, Size: 3454 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-13 7:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-10 8:00 [PATCH 1/1] tools/libs/light: fix BAR memory address truncation Jiqian Chen
2025-10-10 8:19 ` Jan Beulich
2025-10-10 9:02 ` Jürgen Groß
2025-10-10 8:21 ` Jan Beulich
2025-10-13 7:40 ` Oleksii Kurochko
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 NNTP newsgroup(s).