* [PATCH] drm/msm: fix an error code in the ioctl
@ 2019-02-14 7:19 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2019-02-14 7:19 UTC (permalink / raw)
To: Rob Clark
Cc: David Airlie, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
The copy_to/from_user() functions return the number of bytes remaining
to be copied but we should return -EFAULT to the user.
Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
If I were reviewing this patch, I would be suspicous that we don't
return immediately after the first copy_from_user() fails but I'm fairly
sure that is the correct behavior.
drivers/gpu/drm/msm/msm_drv.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b871e2e98129..1d4426cb260d 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
ret = -EINVAL;
break;
}
- ret = copy_from_user(msm_obj->name,
- u64_to_user_ptr(args->value), args->len);
+ if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value),
+ args->len))
+ ret = -EFAULT;
msm_obj->name[args->len] = '\0';
for (i = 0; i < args->len; i++) {
if (!isprint(msm_obj->name[i])) {
@@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
}
args->len = strlen(msm_obj->name);
if (args->value) {
- ret = copy_to_user(u64_to_user_ptr(args->value),
- msm_obj->name, args->len);
+ if (copy_to_user(u64_to_user_ptr(args->value),
+ msm_obj->name, args->len))
+ ret = -EFAULT;
}
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] drm/msm: fix an error code in the ioctl @ 2019-02-14 7:19 ` Dan Carpenter 0 siblings, 0 replies; 8+ messages in thread From: Dan Carpenter @ 2019-02-14 7:19 UTC (permalink / raw) To: Rob Clark Cc: David Airlie, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, kernel-janitors-u79uwXL29TY76Z2rM5mHXA, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW The copy_to/from_user() functions return the number of bytes remaining to be copied but we should return -EFAULT to the user. Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- If I were reviewing this patch, I would be suspicous that we don't return immediately after the first copy_from_user() fails but I'm fairly sure that is the correct behavior. drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index b871e2e98129..1d4426cb260d 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, ret = -EINVAL; break; } - ret = copy_from_user(msm_obj->name, - u64_to_user_ptr(args->value), args->len); + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), + args->len)) + ret = -EFAULT; msm_obj->name[args->len] = '\0'; for (i = 0; i < args->len; i++) { if (!isprint(msm_obj->name[i])) { @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, } args->len = strlen(msm_obj->name); if (args->value) { - ret = copy_to_user(u64_to_user_ptr(args->value), - msm_obj->name, args->len); + if (copy_to_user(u64_to_user_ptr(args->value), + msm_obj->name, args->len)) + ret = -EFAULT; } break; } -- 2.17.1 _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm: fix an error code in the ioctl 2019-02-14 7:19 ` Dan Carpenter @ 2019-02-14 23:16 ` Rob Clark -1 siblings, 0 replies; 8+ messages in thread From: Rob Clark @ 2019-02-14 23:16 UTC (permalink / raw) To: Dan Carpenter Cc: David Airlie, linux-arm-msm, kernel-janitors-u79uwXL29TY76Z2rM5mHXA, dri-devel, Daniel Vetter, freedreno On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > The copy_to/from_user() functions return the number of bytes remaining > to be copied but we should return -EFAULT to the user. > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > If I were reviewing this patch, I would be suspicous that we don't > return immediately after the first copy_from_user() fails but I'm fairly > sure that is the correct behavior. oh, hmm, you are defn right that the current code is incorrect.. Although I guess I wonder if maybe in the -EFAULT case we should set a null char at the end of the # of bytes copied in. I guess the result with your patch as-is is that you'd get part of the new debug name string, and part of the old. Which is maybe not incorrect or worse than truncated new debug name. (It is really mostly just for debugfs after all.) I guess we could copy_from_user() into a temp buffer to leave the old debug name undisturbed in the -EFAULT case, but I'd accept he argument that that would be overkill. BR, -R > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index b871e2e98129..1d4426cb260d 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > ret = -EINVAL; > break; > } > - ret = copy_from_user(msm_obj->name, > - u64_to_user_ptr(args->value), args->len); > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > + args->len)) > + ret = -EFAULT; > msm_obj->name[args->len] = '\0'; > for (i = 0; i < args->len; i++) { > if (!isprint(msm_obj->name[i])) { > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > } > args->len = strlen(msm_obj->name); > if (args->value) { > - ret = copy_to_user(u64_to_user_ptr(args->value), > - msm_obj->name, args->len); > + if (copy_to_user(u64_to_user_ptr(args->value), > + msm_obj->name, args->len)) > + ret = -EFAULT; > } > break; > } > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm: fix an error code in the ioctl @ 2019-02-14 23:16 ` Rob Clark 0 siblings, 0 replies; 8+ messages in thread From: Rob Clark @ 2019-02-14 23:16 UTC (permalink / raw) To: Dan Carpenter Cc: David Airlie, linux-arm-msm, kernel-janitors-u79uwXL29TY76Z2rM5mHXA, dri-devel, Daniel Vetter, freedreno On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > The copy_to/from_user() functions return the number of bytes remaining > to be copied but we should return -EFAULT to the user. > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > If I were reviewing this patch, I would be suspicous that we don't > return immediately after the first copy_from_user() fails but I'm fairly > sure that is the correct behavior. oh, hmm, you are defn right that the current code is incorrect.. Although I guess I wonder if maybe in the -EFAULT case we should set a null char at the end of the # of bytes copied in. I guess the result with your patch as-is is that you'd get part of the new debug name string, and part of the old. Which is maybe not incorrect or worse than truncated new debug name. (It is really mostly just for debugfs after all.) I guess we could copy_from_user() into a temp buffer to leave the old debug name undisturbed in the -EFAULT case, but I'd accept he argument that that would be overkill. BR, -R > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index b871e2e98129..1d4426cb260d 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > ret = -EINVAL; > break; > } > - ret = copy_from_user(msm_obj->name, > - u64_to_user_ptr(args->value), args->len); > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > + args->len)) > + ret = -EFAULT; > msm_obj->name[args->len] = '\0'; > for (i = 0; i < args->len; i++) { > if (!isprint(msm_obj->name[i])) { > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > } > args->len = strlen(msm_obj->name); > if (args->value) { > - ret = copy_to_user(u64_to_user_ptr(args->value), > - msm_obj->name, args->len); > + if (copy_to_user(u64_to_user_ptr(args->value), > + msm_obj->name, args->len)) > + ret = -EFAULT; > } > break; > } > -- > 2.17.1 > _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAF6AEGtw76LLbEnpkbnLwd5Pt1P1_KiCDYcqr9mfrw=nKMo66A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [Freedreno] [PATCH] drm/msm: fix an error code in the ioctl [not found] ` <CAF6AEGtw76LLbEnpkbnLwd5Pt1P1_KiCDYcqr9mfrw=nKMo66A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2019-02-15 16:01 ` Jordan Crouse 0 siblings, 0 replies; 8+ messages in thread From: Jordan Crouse @ 2019-02-15 16:01 UTC (permalink / raw) To: Rob Clark Cc: David Airlie, linux-arm-msm, kernel-janitors-u79uwXL29TY76Z2rM5mHXA, dri-devel, Daniel Vetter, freedreno, Dan Carpenter On Thu, Feb 14, 2019 at 06:16:01PM -0500, Rob Clark wrote: > On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > The copy_to/from_user() functions return the number of bytes remaining > > to be copied but we should return -EFAULT to the user. > > > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > If I were reviewing this patch, I would be suspicous that we don't > > return immediately after the first copy_from_user() fails but I'm fairly > > sure that is the correct behavior. > > oh, hmm, you are defn right that the current code is incorrect.. This one was Boneheaded-by: me. > Although I guess I wonder if maybe in the -EFAULT case we should set a > null char at the end of the # of bytes copied in. I guess the result > with your patch as-is is that you'd get part of the new debug name > string, and part of the old. Which is maybe not incorrect or worse > than truncated new debug name. (It is really mostly just for debugfs > after all.) > I guess we could copy_from_user() into a temp buffer to leave the old > debug name undisturbed in the -EFAULT case, but I'd accept he argument > that that would be overkill. Right. I think on failure we should just truncate the string back to strlen(0) and pretend that nothing happened. I can toss up a patch for that. Jordan > > > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > index b871e2e98129..1d4426cb260d 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.c > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > ret = -EINVAL; > > break; > > } > > - ret = copy_from_user(msm_obj->name, > > - u64_to_user_ptr(args->value), args->len); > > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > > + args->len)) > > + ret = -EFAULT; > > msm_obj->name[args->len] = '\0'; > > for (i = 0; i < args->len; i++) { > > if (!isprint(msm_obj->name[i])) { > > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > } > > args->len = strlen(msm_obj->name); > > if (args->value) { > > - ret = copy_to_user(u64_to_user_ptr(args->value), > > - msm_obj->name, args->len); > > + if (copy_to_user(u64_to_user_ptr(args->value), > > + msm_obj->name, args->len)) > > + ret = -EFAULT; > > } > > break; > > } > > -- > > 2.17.1 > > > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/msm: fix an error code in the ioctl @ 2019-02-15 16:01 ` Jordan Crouse 0 siblings, 0 replies; 8+ messages in thread From: Jordan Crouse @ 2019-02-15 16:01 UTC (permalink / raw) To: Rob Clark Cc: David Airlie, linux-arm-msm, kernel-janitors-u79uwXL29TY76Z2rM5mHXA, dri-devel, Daniel Vetter, freedreno, Dan Carpenter On Thu, Feb 14, 2019 at 06:16:01PM -0500, Rob Clark wrote: > On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > The copy_to/from_user() functions return the number of bytes remaining > > to be copied but we should return -EFAULT to the user. > > > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > If I were reviewing this patch, I would be suspicous that we don't > > return immediately after the first copy_from_user() fails but I'm fairly > > sure that is the correct behavior. > > oh, hmm, you are defn right that the current code is incorrect.. This one was Boneheaded-by: me. > Although I guess I wonder if maybe in the -EFAULT case we should set a > null char at the end of the # of bytes copied in. I guess the result > with your patch as-is is that you'd get part of the new debug name > string, and part of the old. Which is maybe not incorrect or worse > than truncated new debug name. (It is really mostly just for debugfs > after all.) > I guess we could copy_from_user() into a temp buffer to leave the old > debug name undisturbed in the -EFAULT case, but I'd accept he argument > that that would be overkill. Right. I think on failure we should just truncate the string back to strlen(0) and pretend that nothing happened. I can toss up a patch for that. Jordan > > > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > index b871e2e98129..1d4426cb260d 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.c > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > ret = -EINVAL; > > break; > > } > > - ret = copy_from_user(msm_obj->name, > > - u64_to_user_ptr(args->value), args->len); > > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > > + args->len)) > > + ret = -EFAULT; > > msm_obj->name[args->len] = '\0'; > > for (i = 0; i < args->len; i++) { > > if (!isprint(msm_obj->name[i])) { > > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > } > > args->len = strlen(msm_obj->name); > > if (args->value) { > > - ret = copy_to_user(u64_to_user_ptr(args->value), > > - msm_obj->name, args->len); > > + if (copy_to_user(u64_to_user_ptr(args->value), > > + msm_obj->name, args->len)) > > + ret = -EFAULT; > > } > > break; > > } > > -- > > 2.17.1 > > > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Freedreno] [PATCH] drm/msm: fix an error code in the ioctl 2019-02-15 16:01 ` Jordan Crouse @ 2019-02-15 22:31 ` Rob Clark via dri-devel -1 siblings, 0 replies; 8+ messages in thread From: Rob Clark @ 2019-02-15 22:31 UTC (permalink / raw) To: Rob Clark, Dan Carpenter, David Airlie, linux-arm-msm, kernel-janitors, dri-devel, Daniel Vetter, freedreno On Fri, Feb 15, 2019 at 11:01 AM Jordan Crouse <jcrouse@codeaurora.org> wrote: > > On Thu, Feb 14, 2019 at 06:16:01PM -0500, Rob Clark wrote: > > On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > The copy_to/from_user() functions return the number of bytes remaining > > > to be copied but we should return -EFAULT to the user. > > > > > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > If I were reviewing this patch, I would be suspicous that we don't > > > return immediately after the first copy_from_user() fails but I'm fairly > > > sure that is the correct behavior. > > > > oh, hmm, you are defn right that the current code is incorrect.. > > This one was Boneheaded-by: me. come-on, credit where credit is due.. I boneheaded this one, and I don't want to share the credit :-P > > > Although I guess I wonder if maybe in the -EFAULT case we should set a > > null char at the end of the # of bytes copied in. I guess the result > > with your patch as-is is that you'd get part of the new debug name > > string, and part of the old. Which is maybe not incorrect or worse > > than truncated new debug name. (It is really mostly just for debugfs > > after all.) > > > I guess we could copy_from_user() into a temp buffer to leave the old > > debug name undisturbed in the -EFAULT case, but I'd accept he argument > > that that would be overkill. > > Right. I think on failure we should just truncate the string back to strlen(0) > and pretend that nothing happened. I can toss up a patch for that. Hmm, yeah, I think truncating by nulling out first byte is a sane approach.. please remember to include Fixes and Reported-by tags. Thx BR, -R > > Jordan > > > > > > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > > index b871e2e98129..1d4426cb260d 100644 > > > --- a/drivers/gpu/drm/msm/msm_drv.c > > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > > ret = -EINVAL; > > > break; > > > } > > > - ret = copy_from_user(msm_obj->name, > > > - u64_to_user_ptr(args->value), args->len); > > > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > > > + args->len)) > > > + ret = -EFAULT; > > > msm_obj->name[args->len] = '\0'; > > > for (i = 0; i < args->len; i++) { > > > if (!isprint(msm_obj->name[i])) { > > > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > > } > > > args->len = strlen(msm_obj->name); > > > if (args->value) { > > > - ret = copy_to_user(u64_to_user_ptr(args->value), > > > - msm_obj->name, args->len); > > > + if (copy_to_user(u64_to_user_ptr(args->value), > > > + msm_obj->name, args->len)) > > > + ret = -EFAULT; > > > } > > > break; > > > } > > > -- > > > 2.17.1 > > > > > _______________________________________________ > > Freedreno mailing list > > Freedreno@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Freedreno] [PATCH] drm/msm: fix an error code in the ioctl @ 2019-02-15 22:31 ` Rob Clark via dri-devel 0 siblings, 0 replies; 8+ messages in thread From: Rob Clark via dri-devel @ 2019-02-15 22:31 UTC (permalink / raw) To: Rob Clark, Dan Carpenter, David Airlie, linux-arm-msm, kernel-janitors, dri-devel, Daniel Vetter, freedreno On Fri, Feb 15, 2019 at 11:01 AM Jordan Crouse <jcrouse@codeaurora.org> wrote: > > On Thu, Feb 14, 2019 at 06:16:01PM -0500, Rob Clark wrote: > > On Thu, Feb 14, 2019 at 2:19 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > The copy_to/from_user() functions return the number of bytes remaining > > > to be copied but we should return -EFAULT to the user. > > > > > > Fixes: f05c83e77460 ("drm/msm: add uapi to get/set debug name") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > If I were reviewing this patch, I would be suspicous that we don't > > > return immediately after the first copy_from_user() fails but I'm fairly > > > sure that is the correct behavior. > > > > oh, hmm, you are defn right that the current code is incorrect.. > > This one was Boneheaded-by: me. come-on, credit where credit is due.. I boneheaded this one, and I don't want to share the credit :-P > > > Although I guess I wonder if maybe in the -EFAULT case we should set a > > null char at the end of the # of bytes copied in. I guess the result > > with your patch as-is is that you'd get part of the new debug name > > string, and part of the old. Which is maybe not incorrect or worse > > than truncated new debug name. (It is really mostly just for debugfs > > after all.) > > > I guess we could copy_from_user() into a temp buffer to leave the old > > debug name undisturbed in the -EFAULT case, but I'd accept he argument > > that that would be overkill. > > Right. I think on failure we should just truncate the string back to strlen(0) > and pretend that nothing happened. I can toss up a patch for that. Hmm, yeah, I think truncating by nulling out first byte is a sane approach.. please remember to include Fixes and Reported-by tags. Thx BR, -R > > Jordan > > > > > > > drivers/gpu/drm/msm/msm_drv.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > > > index b871e2e98129..1d4426cb260d 100644 > > > --- a/drivers/gpu/drm/msm/msm_drv.c > > > +++ b/drivers/gpu/drm/msm/msm_drv.c > > > @@ -851,8 +851,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > > ret = -EINVAL; > > > break; > > > } > > > - ret = copy_from_user(msm_obj->name, > > > - u64_to_user_ptr(args->value), args->len); > > > + if (copy_from_user(msm_obj->name, u64_to_user_ptr(args->value), > > > + args->len)) > > > + ret = -EFAULT; > > > msm_obj->name[args->len] = '\0'; > > > for (i = 0; i < args->len; i++) { > > > if (!isprint(msm_obj->name[i])) { > > > @@ -868,8 +869,9 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, > > > } > > > args->len = strlen(msm_obj->name); > > > if (args->value) { > > > - ret = copy_to_user(u64_to_user_ptr(args->value), > > > - msm_obj->name, args->len); > > > + if (copy_to_user(u64_to_user_ptr(args->value), > > > + msm_obj->name, args->len)) > > > + ret = -EFAULT; > > > } > > > break; > > > } > > > -- > > > 2.17.1 > > > > > _______________________________________________ > > Freedreno mailing list > > Freedreno@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-15 22:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-14 7:19 [PATCH] drm/msm: fix an error code in the ioctl Dan Carpenter
2019-02-14 7:19 ` Dan Carpenter
2019-02-14 23:16 ` Rob Clark
2019-02-14 23:16 ` Rob Clark
[not found] ` <CAF6AEGtw76LLbEnpkbnLwd5Pt1P1_KiCDYcqr9mfrw=nKMo66A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2019-02-15 16:01 ` [Freedreno] " Jordan Crouse
2019-02-15 16:01 ` Jordan Crouse
2019-02-15 22:31 ` [Freedreno] " Rob Clark
2019-02-15 22:31 ` Rob Clark via dri-devel
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.