* Question regarding swiotlb-xen in Linux kernel
@ 2019-04-18 7:36 ` Juergen Gross
0 siblings, 0 replies; 12+ messages in thread
From: Juergen Gross @ 2019-04-18 7:36 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Boris Ostrovsky
I'm currently investigating a problem related to swiotlb-xen. With a
specific driver a customer is capable to trigger a situation where a
MFN is mapped to multiple dom0 PFNs at the same time. There is no
guest involved, so this is not related to grants.
Wit a debug kernel I have managed to track the inconsistency to a
call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent()
where the region was obviously not contiguous.
xen_swiotlb_free_coherent() contains:
if (((dev_addr + size - 1 <= dma_mask)) ||
range_straddles_page_boundary(phys, size))
xen_destroy_contiguous_region(phys, order);
Shouldn't it be either:
if (((dev_addr + size - 1 <= dma_mask)) &&
!range_straddles_page_boundary(phys, size))
xen_destroy_contiguous_region(phys, order);
or:
if (dev_addr + size - 1 <= dma_mask) {
BUG_ON(range_straddles_page_boundary(phys, size));
xen_destroy_contiguous_region(phys, order);
}
as calling xen_destroy_contiguous_region() with a non-contiguous memory
region is a perfect receipt for a latent crash?
The remaining question is why the driver is calling
xen_swiotlb_free_coherent() for a non-contiguous region, of course.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-18 7:36 ` Juergen Gross 0 siblings, 0 replies; 12+ messages in thread From: Juergen Gross @ 2019-04-18 7:36 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Boris Ostrovsky I'm currently investigating a problem related to swiotlb-xen. With a specific driver a customer is capable to trigger a situation where a MFN is mapped to multiple dom0 PFNs at the same time. There is no guest involved, so this is not related to grants. Wit a debug kernel I have managed to track the inconsistency to a call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() where the region was obviously not contiguous. xen_swiotlb_free_coherent() contains: if (((dev_addr + size - 1 <= dma_mask)) || range_straddles_page_boundary(phys, size)) xen_destroy_contiguous_region(phys, order); Shouldn't it be either: if (((dev_addr + size - 1 <= dma_mask)) && !range_straddles_page_boundary(phys, size)) xen_destroy_contiguous_region(phys, order); or: if (dev_addr + size - 1 <= dma_mask) { BUG_ON(range_straddles_page_boundary(phys, size)); xen_destroy_contiguous_region(phys, order); } as calling xen_destroy_contiguous_region() with a non-contiguous memory region is a perfect receipt for a latent crash? The remaining question is why the driver is calling xen_swiotlb_free_coherent() for a non-contiguous region, of course. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Question regarding swiotlb-xen in Linux kernel @ 2019-04-18 21:09 ` Boris Ostrovsky 0 siblings, 0 replies; 12+ messages in thread From: Boris Ostrovsky @ 2019-04-18 21:09 UTC (permalink / raw) To: Juergen Gross, Konrad Rzeszutek Wilk; +Cc: xen-devel, Joe Jin On 4/18/19 3:36 AM, Juergen Gross wrote: > I'm currently investigating a problem related to swiotlb-xen. With a > specific driver a customer is capable to trigger a situation where a > MFN is mapped to multiple dom0 PFNs at the same time. There is no > guest involved, so this is not related to grants. > > Wit a debug kernel I have managed to track the inconsistency to a > call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() > where the region was obviously not contiguous. > > xen_swiotlb_free_coherent() contains: > > if (((dev_addr + size - 1 <= dma_mask)) || > range_straddles_page_boundary(phys, size)) > xen_destroy_contiguous_region(phys, order); > > Shouldn't it be either: > > if (((dev_addr + size - 1 <= dma_mask)) && > !range_straddles_page_boundary(phys, size)) > xen_destroy_contiguous_region(phys, order); +Joe https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html (The discussion happened in https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) And looks like we dropped it. Or was there a reason we haven't picked it up? -boris > > > or: > > if (dev_addr + size - 1 <= dma_mask) { > BUG_ON(range_straddles_page_boundary(phys, size)); > xen_destroy_contiguous_region(phys, order); > } > > as calling xen_destroy_contiguous_region() with a non-contiguous memory > region is a perfect receipt for a latent crash? > > The remaining question is why the driver is calling > xen_swiotlb_free_coherent() for a non-contiguous region, of course. > > > Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-18 21:09 ` Boris Ostrovsky 0 siblings, 0 replies; 12+ messages in thread From: Boris Ostrovsky @ 2019-04-18 21:09 UTC (permalink / raw) To: Juergen Gross, Konrad Rzeszutek Wilk; +Cc: xen-devel, Joe Jin On 4/18/19 3:36 AM, Juergen Gross wrote: > I'm currently investigating a problem related to swiotlb-xen. With a > specific driver a customer is capable to trigger a situation where a > MFN is mapped to multiple dom0 PFNs at the same time. There is no > guest involved, so this is not related to grants. > > Wit a debug kernel I have managed to track the inconsistency to a > call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() > where the region was obviously not contiguous. > > xen_swiotlb_free_coherent() contains: > > if (((dev_addr + size - 1 <= dma_mask)) || > range_straddles_page_boundary(phys, size)) > xen_destroy_contiguous_region(phys, order); > > Shouldn't it be either: > > if (((dev_addr + size - 1 <= dma_mask)) && > !range_straddles_page_boundary(phys, size)) > xen_destroy_contiguous_region(phys, order); +Joe https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html (The discussion happened in https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) And looks like we dropped it. Or was there a reason we haven't picked it up? -boris > > > or: > > if (dev_addr + size - 1 <= dma_mask) { > BUG_ON(range_straddles_page_boundary(phys, size)); > xen_destroy_contiguous_region(phys, order); > } > > as calling xen_destroy_contiguous_region() with a non-contiguous memory > region is a perfect receipt for a latent crash? > > The remaining question is why the driver is calling > xen_swiotlb_free_coherent() for a non-contiguous region, of course. > > > Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Question regarding swiotlb-xen in Linux kernel @ 2019-04-18 22:31 ` Joe Jin 0 siblings, 0 replies; 12+ messages in thread From: Joe Jin @ 2019-04-18 22:31 UTC (permalink / raw) To: Boris Ostrovsky, Juergen Gross, Konrad Rzeszutek Wilk; +Cc: xen-devel On 4/18/19 2:09 PM, Boris Ostrovsky wrote: > On 4/18/19 3:36 AM, Juergen Gross wrote: >> I'm currently investigating a problem related to swiotlb-xen. With a >> specific driver a customer is capable to trigger a situation where a >> MFN is mapped to multiple dom0 PFNs at the same time. There is no >> guest involved, so this is not related to grants. >> >> Wit a debug kernel I have managed to track the inconsistency to a >> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >> where the region was obviously not contiguous. >> >> xen_swiotlb_free_coherent() contains: >> >> if (((dev_addr + size - 1 <= dma_mask)) || >> range_straddles_page_boundary(phys, size)) >> xen_destroy_contiguous_region(phys, order); >> >> Shouldn't it be either: >> >> if (((dev_addr + size - 1 <= dma_mask)) && >> !range_straddles_page_boundary(phys, size)) >> xen_destroy_contiguous_region(phys, order); > > +Joe > > https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html > > (The discussion happened in > https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) > > And looks like we dropped it. Or was there a reason we haven't picked it up? I remembered the concern was whether memory not from Xen. Thanks, Joe > > > -boris > > >> >> >> or: >> >> if (dev_addr + size - 1 <= dma_mask) { >> BUG_ON(range_straddles_page_boundary(phys, size)); >> xen_destroy_contiguous_region(phys, order); >> } >> >> as calling xen_destroy_contiguous_region() with a non-contiguous memory >> region is a perfect receipt for a latent crash? >> >> The remaining question is why the driver is calling >> xen_swiotlb_free_coherent() for a non-contiguous region, of course. >> >> >> Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-18 22:31 ` Joe Jin 0 siblings, 0 replies; 12+ messages in thread From: Joe Jin @ 2019-04-18 22:31 UTC (permalink / raw) To: Boris Ostrovsky, Juergen Gross, Konrad Rzeszutek Wilk; +Cc: xen-devel On 4/18/19 2:09 PM, Boris Ostrovsky wrote: > On 4/18/19 3:36 AM, Juergen Gross wrote: >> I'm currently investigating a problem related to swiotlb-xen. With a >> specific driver a customer is capable to trigger a situation where a >> MFN is mapped to multiple dom0 PFNs at the same time. There is no >> guest involved, so this is not related to grants. >> >> Wit a debug kernel I have managed to track the inconsistency to a >> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >> where the region was obviously not contiguous. >> >> xen_swiotlb_free_coherent() contains: >> >> if (((dev_addr + size - 1 <= dma_mask)) || >> range_straddles_page_boundary(phys, size)) >> xen_destroy_contiguous_region(phys, order); >> >> Shouldn't it be either: >> >> if (((dev_addr + size - 1 <= dma_mask)) && >> !range_straddles_page_boundary(phys, size)) >> xen_destroy_contiguous_region(phys, order); > > +Joe > > https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html > > (The discussion happened in > https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) > > And looks like we dropped it. Or was there a reason we haven't picked it up? I remembered the concern was whether memory not from Xen. Thanks, Joe > > > -boris > > >> >> >> or: >> >> if (dev_addr + size - 1 <= dma_mask) { >> BUG_ON(range_straddles_page_boundary(phys, size)); >> xen_destroy_contiguous_region(phys, order); >> } >> >> as calling xen_destroy_contiguous_region() with a non-contiguous memory >> region is a perfect receipt for a latent crash? >> >> The remaining question is why the driver is calling >> xen_swiotlb_free_coherent() for a non-contiguous region, of course. >> >> >> Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 5:47 ` Juergen Gross 0 siblings, 0 replies; 12+ messages in thread From: Juergen Gross @ 2019-04-19 5:47 UTC (permalink / raw) To: Joe Jin, Boris Ostrovsky, Konrad Rzeszutek Wilk; +Cc: xen-devel On 19/04/2019 00:31, Joe Jin wrote: > On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >> On 4/18/19 3:36 AM, Juergen Gross wrote: >>> I'm currently investigating a problem related to swiotlb-xen. With a >>> specific driver a customer is capable to trigger a situation where a >>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>> guest involved, so this is not related to grants. >>> >>> Wit a debug kernel I have managed to track the inconsistency to a >>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>> where the region was obviously not contiguous. >>> >>> xen_swiotlb_free_coherent() contains: >>> >>> if (((dev_addr + size - 1 <= dma_mask)) || >>> range_straddles_page_boundary(phys, size)) >>> xen_destroy_contiguous_region(phys, order); >>> >>> Shouldn't it be either: >>> >>> if (((dev_addr + size - 1 <= dma_mask)) && >>> !range_straddles_page_boundary(phys, size)) >>> xen_destroy_contiguous_region(phys, order); >> >> +Joe >> >> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >> >> (The discussion happened in >> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >> >> And looks like we dropped it. Or was there a reason we haven't picked it up? > > I remembered the concern was whether memory not from Xen. The current coding is wrong. I believe we should have at least a WARN_ONCE() in case xen_swiotlb_free_coherent() is being called with non-contiguous memory. And it should not call xen_destroy_contiguous_region() in that case. Joe, did you observe such cases? I'm asking because the backtraces I have give me no clue _why_ the memory was non-contiguous. The calling function allocated the memory via xen_swiotlb_alloc_coherent() and when freeing it again it was not contiguous. Another topic is the question whether we should really call xen_destroy_contiguous_region() when freeing the memory if there was no need to use xen_create_contiguous_region() when allocating it. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 5:47 ` Juergen Gross 0 siblings, 0 replies; 12+ messages in thread From: Juergen Gross @ 2019-04-19 5:47 UTC (permalink / raw) To: Joe Jin, Boris Ostrovsky, Konrad Rzeszutek Wilk; +Cc: xen-devel On 19/04/2019 00:31, Joe Jin wrote: > On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >> On 4/18/19 3:36 AM, Juergen Gross wrote: >>> I'm currently investigating a problem related to swiotlb-xen. With a >>> specific driver a customer is capable to trigger a situation where a >>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>> guest involved, so this is not related to grants. >>> >>> Wit a debug kernel I have managed to track the inconsistency to a >>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>> where the region was obviously not contiguous. >>> >>> xen_swiotlb_free_coherent() contains: >>> >>> if (((dev_addr + size - 1 <= dma_mask)) || >>> range_straddles_page_boundary(phys, size)) >>> xen_destroy_contiguous_region(phys, order); >>> >>> Shouldn't it be either: >>> >>> if (((dev_addr + size - 1 <= dma_mask)) && >>> !range_straddles_page_boundary(phys, size)) >>> xen_destroy_contiguous_region(phys, order); >> >> +Joe >> >> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >> >> (The discussion happened in >> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >> >> And looks like we dropped it. Or was there a reason we haven't picked it up? > > I remembered the concern was whether memory not from Xen. The current coding is wrong. I believe we should have at least a WARN_ONCE() in case xen_swiotlb_free_coherent() is being called with non-contiguous memory. And it should not call xen_destroy_contiguous_region() in that case. Joe, did you observe such cases? I'm asking because the backtraces I have give me no clue _why_ the memory was non-contiguous. The calling function allocated the memory via xen_swiotlb_alloc_coherent() and when freeing it again it was not contiguous. Another topic is the question whether we should really call xen_destroy_contiguous_region() when freeing the memory if there was no need to use xen_create_contiguous_region() when allocating it. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 14:40 ` Joe Jin 0 siblings, 0 replies; 12+ messages in thread From: Joe Jin @ 2019-04-19 14:40 UTC (permalink / raw) To: Juergen Gross, Boris Ostrovsky, Konrad Rzeszutek Wilk; +Cc: xen-devel On 4/18/19 10:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. I agree with you, your patch same with I sent before, I'm good to have it. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. It's potential issue, if alloc/free is not in pairs, it will be a problem. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. Not sure if you have commit 7250f422 "xen-swiotlb: use actually allocated size on check physical continuous"? without this commit, it's possible, after apply the commit, we did not hit such kind cases. Thanks, Joe > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. > > > Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 14:40 ` Joe Jin 0 siblings, 0 replies; 12+ messages in thread From: Joe Jin @ 2019-04-19 14:40 UTC (permalink / raw) To: Juergen Gross, Boris Ostrovsky, Konrad Rzeszutek Wilk; +Cc: xen-devel On 4/18/19 10:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. I agree with you, your patch same with I sent before, I'm good to have it. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. It's potential issue, if alloc/free is not in pairs, it will be a problem. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. Not sure if you have commit 7250f422 "xen-swiotlb: use actually allocated size on check physical continuous"? without this commit, it's possible, after apply the commit, we did not hit such kind cases. Thanks, Joe > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. > > > Juergen > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 15:28 ` Dongli Zhang 0 siblings, 0 replies; 12+ messages in thread From: Dongli Zhang @ 2019-04-19 15:28 UTC (permalink / raw) To: Juergen Gross; +Cc: xen-devel, Boris Ostrovsky, Joe Jin, Konrad Rzeszutek Wilk On 04/19/2019 01:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. What would happen if guest domain is malicious? That is, guest (including dom0) my allocate unlimited MFN continuous memory and never exchange them back to xen. Would MFN continuous memory be used up? As a result, xen may not be able to boot new VM. Is this a sort of DoS attack? Dongli Zhang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] Question regarding swiotlb-xen in Linux kernel @ 2019-04-19 15:28 ` Dongli Zhang 0 siblings, 0 replies; 12+ messages in thread From: Dongli Zhang @ 2019-04-19 15:28 UTC (permalink / raw) To: Juergen Gross; +Cc: xen-devel, Boris Ostrovsky, Joe Jin, Konrad Rzeszutek Wilk On 04/19/2019 01:47 PM, Juergen Gross wrote: > On 19/04/2019 00:31, Joe Jin wrote: >> On 4/18/19 2:09 PM, Boris Ostrovsky wrote: >>> On 4/18/19 3:36 AM, Juergen Gross wrote: >>>> I'm currently investigating a problem related to swiotlb-xen. With a >>>> specific driver a customer is capable to trigger a situation where a >>>> MFN is mapped to multiple dom0 PFNs at the same time. There is no >>>> guest involved, so this is not related to grants. >>>> >>>> Wit a debug kernel I have managed to track the inconsistency to a >>>> call of xen_destroy_contiguous_region() from xen_swiotlb_free_coherent() >>>> where the region was obviously not contiguous. >>>> >>>> xen_swiotlb_free_coherent() contains: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) || >>>> range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>>> >>>> Shouldn't it be either: >>>> >>>> if (((dev_addr + size - 1 <= dma_mask)) && >>>> !range_straddles_page_boundary(phys, size)) >>>> xen_destroy_contiguous_region(phys, order); >>> >>> +Joe >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01920.html >>> >>> (The discussion happened in >>> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01814.html) >>> >>> And looks like we dropped it. Or was there a reason we haven't picked it up? >> >> I remembered the concern was whether memory not from Xen. > > The current coding is wrong. > > I believe we should have at least a WARN_ONCE() in case > xen_swiotlb_free_coherent() is being called with non-contiguous memory. > And it should not call xen_destroy_contiguous_region() in that case. > > Joe, did you observe such cases? I'm asking because the backtraces I > have give me no clue _why_ the memory was non-contiguous. The calling > function allocated the memory via xen_swiotlb_alloc_coherent() and when > freeing it again it was not contiguous. > > Another topic is the question whether we should really call > xen_destroy_contiguous_region() when freeing the memory if there was no > need to use xen_create_contiguous_region() when allocating it. What would happen if guest domain is malicious? That is, guest (including dom0) my allocate unlimited MFN continuous memory and never exchange them back to xen. Would MFN continuous memory be used up? As a result, xen may not be able to boot new VM. Is this a sort of DoS attack? Dongli Zhang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-19 15:29 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-18 7:36 Question regarding swiotlb-xen in Linux kernel Juergen Gross 2019-04-18 7:36 ` [Xen-devel] " Juergen Gross 2019-04-18 21:09 ` Boris Ostrovsky 2019-04-18 21:09 ` [Xen-devel] " Boris Ostrovsky 2019-04-18 22:31 ` Joe Jin 2019-04-18 22:31 ` [Xen-devel] " Joe Jin 2019-04-19 5:47 ` Juergen Gross 2019-04-19 5:47 ` [Xen-devel] " Juergen Gross 2019-04-19 14:40 ` Joe Jin 2019-04-19 14:40 ` [Xen-devel] " Joe Jin 2019-04-19 15:28 ` Dongli Zhang 2019-04-19 15:28 ` [Xen-devel] " Dongli Zhang
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.