Messages in this thread | | | Subject | Re: [PATCH 11/12] device-dax: Add dis-contiguous resource support | From | Joao Martins <> | Date | Tue, 12 May 2020 15:36:50 +0100 |
| |
On 3/23/20 11:55 PM, Dan Williams wrote: > @@ -561,13 +580,26 @@ static int __alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start, > if (start == U64_MAX) > return -EINVAL; > > + ranges = krealloc(dev_dax->ranges, sizeof(*ranges) > + * (dev_dax->nr_range + 1), GFP_KERNEL); > + if (!ranges) > + return -ENOMEM; > + > alloc = __request_region(res, start, size, dev_name(dev), 0); > - if (!alloc) > + if (!alloc) { > + kfree(ranges); > return -ENOMEM; > + }
Noticed this yesterday while looking at alloc_dev_dax_range().
Is it correct to free @ranges here on __request_region failure?
IIUC krealloc() would free dev_dax->ranges if it succeeds, leaving us without any valid ranges if __request_region failure case indeed frees @ranges. These @ranges are being used afterwards when we delete the interface and free the assigned regions. Perhaps we should remove the kfree() above and set dev_dax->ranges instead before __request_region; or alternatively change the call order between krealloc and __request_region? FWIW, krealloc checks if the object being reallocated already meets the requested size, so perhaps there's no harm with going with the former.
Joao
| |