* [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf
@ 2022-07-11 10:38 Anthony PERARD
2022-07-11 10:44 ` Juergen Gross
0 siblings, 1 reply; 4+ messages in thread
From: Anthony PERARD @ 2022-07-11 10:38 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, G.R., Wei Liu, Juergen Gross
libxl__xs_directory() can potentially return NULL without setting `n`.
As `n` isn't initialised, we need to check libxl__xs_directory()
return value before checking `n`. Otherwise, `n` might be non-zero
with `bdfs` NULL which would lead to a segv.
Reported-by: "G.R." <firemeteor@users.sourceforge.net>
Fixes: 57bff091f4 ("libxl: add 'name' field to 'libxl_device_pci' in the IDL...")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Hi G.R., you've reported a segv in name2bdf(), and that the only
potential segv I've found. I hope it's the same one as you've
experienced!
---
tools/libs/light/libxl_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 96f88795b6..f4c4f17545 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -859,7 +859,7 @@ static int name2bdf(libxl__gc *gc, libxl_device_pci *pci)
int rc = ERROR_NOTFOUND;
bdfs = libxl__xs_directory(gc, XBT_NULL, PCI_INFO_PATH, &n);
- if (!n)
+ if (!bdfs || !n)
goto out;
for (i = 0; i < n; i++) {
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf
2022-07-11 10:38 [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf Anthony PERARD
@ 2022-07-11 10:44 ` Juergen Gross
2022-07-11 15:35 ` G.R.
0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2022-07-11 10:44 UTC (permalink / raw)
To: Anthony PERARD, xen-devel; +Cc: G.R., Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 571 bytes --]
On 11.07.22 12:38, Anthony PERARD wrote:
> libxl__xs_directory() can potentially return NULL without setting `n`.
> As `n` isn't initialised, we need to check libxl__xs_directory()
> return value before checking `n`. Otherwise, `n` might be non-zero
> with `bdfs` NULL which would lead to a segv.
>
> Reported-by: "G.R." <firemeteor@users.sourceforge.net>
> Fixes: 57bff091f4 ("libxl: add 'name' field to 'libxl_device_pci' in the IDL...")
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf
2022-07-11 10:44 ` Juergen Gross
@ 2022-07-11 15:35 ` G.R.
2022-07-11 15:44 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: G.R. @ 2022-07-11 15:35 UTC (permalink / raw)
To: Juergen Gross; +Cc: Anthony PERARD, xen-devel, Wei Liu
On Mon, Jul 11, 2022 at 7:03 PM Juergen Gross <jgross@suse.com> wrote:
>
> On 11.07.22 12:38, Anthony PERARD wrote:
> > libxl__xs_directory() can potentially return NULL without setting `n`.
> > As `n` isn't initialised, we need to check libxl__xs_directory()
> > return value before checking `n`. Otherwise, `n` might be non-zero
> > with `bdfs` NULL which would lead to a segv.
> >
> > Reported-by: "G.R." <firemeteor@users.sourceforge.net>
> > Fixes: 57bff091f4 ("libxl: add 'name' field to 'libxl_device_pci' in the IDL...")
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
Hi Anthony,
I can confirm that the change fixed the segment fault issue I observed
on 4.16.1 release.
However, the behavior is still not entirely clean after the fix. But
that's probably a separate issue.
I have two devices assigned to pckback through kernel command-line.
When I attempted to use pci-assignable-remove on the SSD that caused
me a lot of trouble, it fails on the first attempt but works fine
later...
root@gaia:~# xl pci-assignable-list
0000:00:17.0
0000:05:00.0
root@gaia:~# xl pci-assignable-remove 05:00.0
libxl: error: libxl_pci.c:906:libxl__device_pci_assignable_remove:
failed to de-quarantine 0000:05:00.0
root@gaia:~# xl pci-assignable-list
0000:00:17.0
0000:05:00.0
root@gaia:~# xl pci-assignable-add 05:00.0
libxl: warning: libxl_pci.c:802:libxl__device_pci_assignable_add:
0000:05:00.0 already assigned to pciback
root@gaia:~# xl pci-assignable-list
0000:00:17.0
0000:05:00.0
root@gaia:~# xl pci-assignable-remove 05:00.0
root@gaia:~# xl pci-assignable-list
0000:00:17.0
root@gaia:~# xl pci-assignable-add 05:00.0
libxl: warning: libxl_pci.c:822:libxl__device_pci_assignable_add:
0000:05:00.0 not bound to a driver, will not be rebound.
root@gaia:~# xl pci-assignable-list
0000:00:17.0
0000:05:00.0
I'm no longer in a debug environment, so cannot collect more debug
info right now.
Thanks,
G.R.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf
2022-07-11 15:35 ` G.R.
@ 2022-07-11 15:44 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2022-07-11 15:44 UTC (permalink / raw)
To: G.R.; +Cc: Anthony PERARD, xen-devel, Wei Liu, Juergen Gross
On 11.07.2022 17:35, G.R. wrote:
> On Mon, Jul 11, 2022 at 7:03 PM Juergen Gross <jgross@suse.com> wrote:
>>
>> On 11.07.22 12:38, Anthony PERARD wrote:
>>> libxl__xs_directory() can potentially return NULL without setting `n`.
>>> As `n` isn't initialised, we need to check libxl__xs_directory()
>>> return value before checking `n`. Otherwise, `n` might be non-zero
>>> with `bdfs` NULL which would lead to a segv.
>>>
>>> Reported-by: "G.R." <firemeteor@users.sourceforge.net>
>>> Fixes: 57bff091f4 ("libxl: add 'name' field to 'libxl_device_pci' in the IDL...")
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> Reviewed-by: Juergen Gross <jgross@suse.com>
>
> I can confirm that the change fixed the segment fault issue I observed
> on 4.16.1 release.
I'll take the liberty and transform this into a Tested-by:.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-11 15:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 10:38 [XEN PATCH] libxl: Check return value of libxl__xs_directory in name2bdf Anthony PERARD
2022-07-11 10:44 ` Juergen Gross
2022-07-11 15:35 ` G.R.
2022-07-11 15:44 ` Jan Beulich
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.