On 11/09/2018 12:01 PM, Halil Pasic wrote:
On Fri, 9 Nov 2018 09:49:22 -0500
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
On 11/09/2018 05:45 AM, Cornelia Huck wrote:
On Fri, 9 Nov 2018 03:39:28 +0100
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
If pfn_array_alloc fails somehow, we need to release the
pfn_array_table that was malloc'd earlier.
Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
---
drivers/s390/cio/vfio_ccw_cp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/cio/vfio_ccw_cp.c
b/drivers/s390/cio/vfio_ccw_cp.c index fd77e46eb3b2..ef5ab45d94b3
100644 --- a/drivers/s390/cio/vfio_ccw_cp.c
+++ b/drivers/s390/cio/vfio_ccw_cp.c
@@ -528,7 +528,7 @@ static int ccwchain_fetch_direct(struct
ccwchain *chain,
ret = pfn_array_alloc_pin(pat->pat_pa, cp->mdev,
ccw->cda, ccw->count); if (ret < 0)
- goto out_init;
+ goto out_unpin;
/* Translate this direct ccw to a idal ccw. */
idaws = kcalloc(ret, sizeof(*idaws), GFP_DMA |
GFP_KERNEL);
It's a bit confusing that this will also call vfio_unpin_pages()
even though there should not be any pinned pages at that point in
time; but from what I see, it should not hurt, so this patch is
fine.
Yeah, confusing indeed. The problem today is that we don't undo the
pfn_array_table_init() call that happened prior to this error, and so
there would be a leak of the pat->pat_pa memory. So going to
out_init is certainly not right.
In the pfn_array patches later, I do conditionally call
vfio_unpin_pages() based on the contents of pfn_array->pa_iova, to
avoid the scenario you describe.
- Eric
Quite confusing and generally awful. I will try to figure out the
before-after on a series level, and then consider the individual
patches in detail. The in between states are predestined to look awful
because of the current state.
Regards,
Halil
The fix is correct but yeah we are unpinning pages when we shouldn't
have anymore pinned pages.
Maybe add an extra check in pfn_array_unpin_free to see if pa_iova_pfn
is null? I don't know if it would make it even more confusing :)
Thanks
Farhan
