| Message ID | bd6a37e2-438d-4292-81e8-5a8f0b10d647@kili.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/amdgpu: Fix up locking etc in amdgpu_debugfs_gprwave_ioctl() | expand |
Applied. Thanks! Alex On Thu, May 25, 2023 at 4:05 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > There are two bugs here. > 1) Drop the lock if copy_from_user() fails. > 2) If the copy fails then the correct error code is -EFAULT instead of > -EINVAL. > > I also broke up the long line and changed "sizeof rd->id" to > "sizeof(rd->id)". > > Fixes: 164fb2940933 ("drm/amd/amdgpu: Update debugfs for XCC support (v3)") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > index c657bed350ac..56e89e76ff17 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > @@ -478,15 +478,16 @@ static ssize_t amdgpu_debugfs_gprwave_read(struct file *f, char __user *buf, siz > static long amdgpu_debugfs_gprwave_ioctl(struct file *f, unsigned int cmd, unsigned long data) > { > struct amdgpu_debugfs_gprwave_data *rd = f->private_data; > - int r; > + int r = 0; > > mutex_lock(&rd->lock); > > switch (cmd) { > case AMDGPU_DEBUGFS_GPRWAVE_IOC_SET_STATE: > - r = copy_from_user(&rd->id, (struct amdgpu_debugfs_gprwave_iocdata *)data, sizeof rd->id); > - if (r) > - return r ? -EINVAL : 0; > + if (copy_from_user(&rd->id, > + (struct amdgpu_debugfs_gprwave_iocdata *)data, > + sizeof(rd->id))) > + r = -EFAULT; > goto done; > default: > r = -EINVAL; > -- > 2.39.2 >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c index c657bed350ac..56e89e76ff17 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c @@ -478,15 +478,16 @@ static ssize_t amdgpu_debugfs_gprwave_read(struct file *f, char __user *buf, siz static long amdgpu_debugfs_gprwave_ioctl(struct file *f, unsigned int cmd, unsigned long data) { struct amdgpu_debugfs_gprwave_data *rd = f->private_data; - int r; + int r = 0; mutex_lock(&rd->lock); switch (cmd) { case AMDGPU_DEBUGFS_GPRWAVE_IOC_SET_STATE: - r = copy_from_user(&rd->id, (struct amdgpu_debugfs_gprwave_iocdata *)data, sizeof rd->id); - if (r) - return r ? -EINVAL : 0; + if (copy_from_user(&rd->id, + (struct amdgpu_debugfs_gprwave_iocdata *)data, + sizeof(rd->id))) + r = -EFAULT; goto done; default: r = -EINVAL;
There are two bugs here. 1) Drop the lock if copy_from_user() fails. 2) If the copy fails then the correct error code is -EFAULT instead of -EINVAL. I also broke up the long line and changed "sizeof rd->id" to "sizeof(rd->id)". Fixes: 164fb2940933 ("drm/amd/amdgpu: Update debugfs for XCC support (v3)") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)