[bug report] fpga: dfl: fme: align PR buffer size per PR datawidth

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Wu Hao,

The patch 69416739ee36: "fpga: dfl: fme: align PR buffer size per PR
datawidth" from Jun 27, 2019, leads to the following Smatch static
checker warning:

	drivers/fpga/dfl-fme-pr.c:104 fme_pr()
	warn: potential integer overflow from user '((port_pr.buffer_size)) + (((4)) - 1)' [local copy]

drivers/fpga/dfl-fme-pr.c
    66 static int fme_pr(struct platform_device *pdev, unsigned long arg)
    67 {
    68         struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
    69         void __user *argp = (void __user *)arg;
    70         struct dfl_fpga_fme_port_pr port_pr;
    71         struct fpga_image_info *info;
    72         struct fpga_region *region;
    73         void __iomem *fme_hdr;
    74         struct dfl_fme *fme;
    75         unsigned long minsz;
    76         void *buf = NULL;
    77         size_t length;
    78         int ret = 0;
    79         u64 v;
    80 
    81         minsz = offsetofend(struct dfl_fpga_fme_port_pr, buffer_address);
    82 
    83         if (copy_from_user(&port_pr, argp, minsz))
    84                 return -EFAULT;
    85 
    86         if (port_pr.argsz < minsz || port_pr.flags)
    87                 return -EINVAL;
    88 
    89         /* get fme header region */
    90         fme_hdr = dfl_get_feature_ioaddr_by_id(&pdev->dev,
    91                                                FME_FEATURE_ID_HEADER);
    92 
    93         /* check port id */
    94         v = readq(fme_hdr + FME_HDR_CAP);
    95         if (port_pr.port_id >= FIELD_GET(FME_CAP_NUM_PORTS, v)) {
    96                 dev_dbg(&pdev->dev, "port number more than maximum\n");
    97                 return -EINVAL;
    98         }
    99 
    100         /*
    101          * align PR buffer per PR bandwidth, as HW ignores the extra padding
    102          * data automatically.
    103          */
--> 104         length = ALIGN(port_pr.buffer_size, 4);

The ALIGN() macro can wrap to zero if it's >= UINT_MAX - 3.

    105 
    106         buf = vmalloc(length);

It's sort of harmless-ish because vmalloc() will WARN_ON_ONCE() if you
pass it a zero and return NULL.  But we could try to prevent the stack
trace.

    107         if (!buf)
    108                 return -ENOMEM;
    109 
    110         if (copy_from_user(buf,
    111                            (void __user *)(unsigned long)port_pr.buffer_address,
    112                            port_pr.buffer_size)) {

This doesn't zero out the padding, so it might be flagged as an info
leak depending on who is reviewing.

    113                 ret = -EFAULT;
    114                 goto free_exit;
    115         }
    116 
    117         /* prepare fpga_image_info for PR */
    118         info = fpga_image_info_alloc(&pdev->dev);
    119         if (!info) {
    120                 ret = -ENOMEM;
    121                 goto free_exit;
    122         }
    123 
    124         info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
    125 
    126         mutex_lock(&pdata->lock);
    127         fme = dfl_fpga_pdata_get_private(pdata);
    128         /* fme device has been unregistered. */
    129         if (!fme) {
    130                 ret = -EINVAL;
    131                 goto unlock_exit;
    132         }
    133 
    134         region = dfl_fme_region_find(fme, port_pr.port_id);
    135         if (!region) {
    136                 ret = -EINVAL;
    137                 goto unlock_exit;
    138         }
    139 
    140         fpga_image_info_free(region->info);
    141 
    142         info->buf = buf;
    143         info->count = length;
    144         info->region_id = port_pr.port_id;
    145         region->info = info;
    146 
    147         ret = fpga_region_program_fpga(region);
    148 
    149         /*
    150          * it allows userspace to reset the PR region's logic by disabling and
    151          * reenabling the bridge to clear things out between acceleration runs.
    152          * so no need to hold the bridges after partial reconfiguration.
    153          */
    154         if (region->get_bridges)
    155                 fpga_bridges_put(&region->bridge_list);
    156 
    157         put_device(&region->dev);
    158 unlock_exit:
    159         mutex_unlock(&pdata->lock);
    160 free_exit:
    161         vfree(buf);
    162         return ret;
    163 }

regards,
dan carpenter