Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Integer overflow in __dev_dax_resize causes >2GB allocations to be treated as errors, leaking DAX resources on 32-bit systems. --
commit abac505d7b066ca0edac0e8d7df59c037cc29e89 Author: Ira Weiny <[email protected]> dax/bus: Factor out dev dax resize logic This commit factors out the device dax resize logic to prepare for dynamic capacity regions. It introduces a helper function that allocates ranges using a specified parent resource, returning the successfully allocated size. > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index 95683dc8fcd0c..ffa6b303fc9bd 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c [ ... ] > +static ssize_t dev_dax_resize_static(struct resource *parent, > + struct dev_dax *dev_dax, > + resource_size_t to_alloc) > { > struct resource *res, *first; > int rc; > > + first = parent->child; > + if (!first) { > + rc = alloc_dev_dax_range(parent, dev_dax, > + parent->start, to_alloc, NULL); > + if (rc) > + return rc; > + return to_alloc; > + } > > for (res = first; res; res = res->sibling) { > struct resource *next = res->sibling; > + resource_size_t alloc; > > /* space at the beginning of the region */ > + if (res == first && res->start > parent->start) { > + alloc = min(res->start - parent->start, to_alloc); > + rc = alloc_dev_dax_range(parent, dev_dax, > + parent->start, alloc, NULL); > + if (rc) > + return rc; > + return alloc; [Severity: High] On 32-bit systems where ssize_t is a 32-bit signed integer and resource_size_t can be an unsigned 64-bit value, does returning a 2GB or larger alloc size cause an integer overflow? If alloc is >= 2GB, returning it as ssize_t could wrap to a negative number or zero if truncated from a larger type. [ ... ] > +static ssize_t dev_dax_resize(struct dax_region *dax_region, > + struct dev_dax *dev_dax, resource_size_t size) > +{ [ ... ] > +retry: > + alloc = dev_dax_resize_static(&dax_region->res, dev_dax, to_alloc); > + if (alloc < 0) > + return alloc; [Severity: High] If a large allocation overflows into a negative ssize_t, wouldn't this caller evaluate alloc < 0 as true and mistakenly treat the successful allocation as an error? Because the successfully mapped chunk was already added to dev_dax->ranges in dev_dax_resize_static(), aborting the resize here without unwinding the newly allocated range seems like it would permanently leak the DAX space and leave the region in an inconsistent state. > + if (alloc == 0) > + return -ENOSPC; > to_alloc -= alloc; > if (to_alloc) > goto retry; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=23
