* [bug report] fpga: dfl: fme: align PR buffer size per PR datawidth
@ 2022-02-22 8:48 Dan Carpenter
2022-03-01 6:20 ` Wu, Hao
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-02-22 8:48 UTC (permalink / raw)
To: hao.wu; +Cc: linux-fpga
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(®ion->bridge_list);
156
157 put_device(®ion->dev);
158 unlock_exit:
159 mutex_unlock(&pdata->lock);
160 free_exit:
161 vfree(buf);
162 return ret;
163 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [bug report] fpga: dfl: fme: align PR buffer size per PR datawidth
2022-02-22 8:48 [bug report] fpga: dfl: fme: align PR buffer size per PR datawidth Dan Carpenter
@ 2022-03-01 6:20 ` Wu, Hao
0 siblings, 0 replies; 2+ messages in thread
From: Wu, Hao @ 2022-03-01 6:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-fpga@vger.kernel.org
> Subject: [bug report] fpga: dfl: fme: align PR buffer size per PR datawidth
>
> 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]
>
Thanks for reporting this, will add checking for it.
Hao
> 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(®ion->bridge_list);
> 156
> 157 put_device(®ion->dev);
> 158 unlock_exit:
> 159 mutex_unlock(&pdata->lock);
> 160 free_exit:
> 161 vfree(buf);
> 162 return ret;
> 163 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-01 6:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-22 8:48 [bug report] fpga: dfl: fme: align PR buffer size per PR datawidth Dan Carpenter
2022-03-01 6:20 ` Wu, Hao
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.