]>
Commit | Line | Data |
---|---|---|
acec1a87 GKH |
1 | From foo@baz Wed Apr 4 17:30:18 CEST 2018 |
2 | From: Mauro Carvalho Chehab <mchehab@s-opensource.com> | |
3 | Date: Wed, 28 Mar 2018 15:12:34 -0300 | |
4 | Subject: media: v4l2-ctrls: fix sparse warning | |
5 | To: Linux Media Mailing List <linux-media@vger.kernel.org>, stable@vger.kernel.org | |
6 | Cc: Hans Verkuil <hans.verkuil@cisco.com>, Mauro Carvalho Chehab <mchehab@infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Mauro Carvalho Chehab <mchehab@s-opensource.com> | |
7 | Message-ID: <d53d22eb0ea4cdbcb2e7f02d789a01892d8c36cf.1522260310.git.mchehab@s-opensource.com> | |
8 | ||
9 | From: Hans Verkuil <hans.verkuil@cisco.com> | |
10 | ||
11 | The warning is simple: | |
12 | ||
13 | drivers/media/v4l2-core/v4l2-ctrls.c:1685:15: warning: incorrect type in assignment (different address spaces) | |
14 | ||
15 | but the fix isn't. | |
16 | ||
17 | The core problem was that the conversion from user to kernelspace was | |
18 | done at too low a level and that needed to be moved up. That made it possible | |
19 | to drop pointers to v4l2_ext_control from set_ctrl and validate_new and | |
20 | clean up this sparse warning because those functions now always operate | |
21 | on kernelspace pointers. | |
22 | ||
23 | Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> | |
24 | Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> | |
25 | Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> | |
26 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
27 | --- | |
28 | drivers/media/v4l2-core/v4l2-ctrls.c | 87 ++++++++++++++++++++--------------- | |
29 | 1 file changed, 52 insertions(+), 35 deletions(-) | |
30 | ||
31 | --- a/drivers/media/v4l2-core/v4l2-ctrls.c | |
32 | +++ b/drivers/media/v4l2-core/v4l2-ctrls.c | |
33 | @@ -1668,10 +1668,8 @@ static int check_range(enum v4l2_ctrl_ty | |
34 | } | |
35 | ||
36 | /* Validate a new control */ | |
37 | -static int validate_new(const struct v4l2_ctrl *ctrl, | |
38 | - struct v4l2_ext_control *c) | |
39 | +static int validate_new(const struct v4l2_ctrl *ctrl, union v4l2_ctrl_ptr p_new) | |
40 | { | |
41 | - union v4l2_ctrl_ptr ptr; | |
42 | unsigned idx; | |
43 | int err = 0; | |
44 | ||
45 | @@ -1684,19 +1682,14 @@ static int validate_new(const struct v4l | |
46 | case V4L2_CTRL_TYPE_BOOLEAN: | |
47 | case V4L2_CTRL_TYPE_BUTTON: | |
48 | case V4L2_CTRL_TYPE_CTRL_CLASS: | |
49 | - ptr.p_s32 = &c->value; | |
50 | - return ctrl->type_ops->validate(ctrl, 0, ptr); | |
51 | - | |
52 | case V4L2_CTRL_TYPE_INTEGER64: | |
53 | - ptr.p_s64 = &c->value64; | |
54 | - return ctrl->type_ops->validate(ctrl, 0, ptr); | |
55 | + return ctrl->type_ops->validate(ctrl, 0, p_new); | |
56 | default: | |
57 | break; | |
58 | } | |
59 | } | |
60 | - ptr.p = c->ptr; | |
61 | - for (idx = 0; !err && idx < c->size / ctrl->elem_size; idx++) | |
62 | - err = ctrl->type_ops->validate(ctrl, idx, ptr); | |
63 | + for (idx = 0; !err && idx < ctrl->elems; idx++) | |
64 | + err = ctrl->type_ops->validate(ctrl, idx, p_new); | |
65 | return err; | |
66 | } | |
67 | ||
68 | @@ -3020,6 +3013,7 @@ static int validate_ctrls(struct v4l2_ex | |
69 | cs->error_idx = cs->count; | |
70 | for (i = 0; i < cs->count; i++) { | |
71 | struct v4l2_ctrl *ctrl = helpers[i].ctrl; | |
72 | + union v4l2_ctrl_ptr p_new; | |
73 | ||
74 | cs->error_idx = i; | |
75 | ||
76 | @@ -3033,7 +3027,17 @@ static int validate_ctrls(struct v4l2_ex | |
77 | best-effort to avoid that. */ | |
78 | if (set && (ctrl->flags & V4L2_CTRL_FLAG_GRABBED)) | |
79 | return -EBUSY; | |
80 | - ret = validate_new(ctrl, &cs->controls[i]); | |
81 | + /* | |
82 | + * Skip validation for now if the payload needs to be copied | |
83 | + * from userspace into kernelspace. We'll validate those later. | |
84 | + */ | |
85 | + if (ctrl->is_ptr) | |
86 | + continue; | |
87 | + if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64) | |
88 | + p_new.p_s64 = &cs->controls[i].value64; | |
89 | + else | |
90 | + p_new.p_s32 = &cs->controls[i].value; | |
91 | + ret = validate_new(ctrl, p_new); | |
92 | if (ret) | |
93 | return ret; | |
94 | } | |
95 | @@ -3128,7 +3132,11 @@ static int try_set_ext_ctrls(struct v4l2 | |
96 | /* Copy the new caller-supplied control values. | |
97 | user_to_new() sets 'is_new' to 1. */ | |
98 | do { | |
99 | - ret = user_to_new(cs->controls + idx, helpers[idx].ctrl); | |
100 | + struct v4l2_ctrl *ctrl = helpers[idx].ctrl; | |
101 | + | |
102 | + ret = user_to_new(cs->controls + idx, ctrl); | |
103 | + if (!ret && ctrl->is_ptr) | |
104 | + ret = validate_new(ctrl, ctrl->p_new); | |
105 | idx = helpers[idx].next; | |
106 | } while (!ret && idx); | |
107 | ||
108 | @@ -3178,10 +3186,10 @@ int v4l2_subdev_s_ext_ctrls(struct v4l2_ | |
109 | EXPORT_SYMBOL(v4l2_subdev_s_ext_ctrls); | |
110 | ||
111 | /* Helper function for VIDIOC_S_CTRL compatibility */ | |
112 | -static int set_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, | |
113 | - struct v4l2_ext_control *c, u32 ch_flags) | |
114 | +static int set_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 ch_flags) | |
115 | { | |
116 | struct v4l2_ctrl *master = ctrl->cluster[0]; | |
117 | + int ret; | |
118 | int i; | |
119 | ||
120 | /* Reset the 'is_new' flags of the cluster */ | |
121 | @@ -3189,8 +3197,9 @@ static int set_ctrl(struct v4l2_fh *fh, | |
122 | if (master->cluster[i]) | |
123 | master->cluster[i]->is_new = 0; | |
124 | ||
125 | - if (c) | |
126 | - user_to_new(c, ctrl); | |
127 | + ret = validate_new(ctrl, ctrl->p_new); | |
128 | + if (ret) | |
129 | + return ret; | |
130 | ||
131 | /* For autoclusters with volatiles that are switched from auto to | |
132 | manual mode we have to update the current volatile values since | |
133 | @@ -3207,15 +3216,14 @@ static int set_ctrl(struct v4l2_fh *fh, | |
134 | static int set_ctrl_lock(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, | |
135 | struct v4l2_ext_control *c) | |
136 | { | |
137 | - int ret = validate_new(ctrl, c); | |
138 | + int ret; | |
139 | ||
140 | - if (!ret) { | |
141 | - v4l2_ctrl_lock(ctrl); | |
142 | - ret = set_ctrl(fh, ctrl, c, 0); | |
143 | - if (!ret) | |
144 | - cur_to_user(c, ctrl); | |
145 | - v4l2_ctrl_unlock(ctrl); | |
146 | - } | |
147 | + v4l2_ctrl_lock(ctrl); | |
148 | + user_to_new(c, ctrl); | |
149 | + ret = set_ctrl(fh, ctrl, 0); | |
150 | + if (!ret) | |
151 | + cur_to_user(c, ctrl); | |
152 | + v4l2_ctrl_unlock(ctrl); | |
153 | return ret; | |
154 | } | |
155 | ||
156 | @@ -3223,7 +3231,7 @@ int v4l2_s_ctrl(struct v4l2_fh *fh, stru | |
157 | struct v4l2_control *control) | |
158 | { | |
159 | struct v4l2_ctrl *ctrl = v4l2_ctrl_find(hdl, control->id); | |
160 | - struct v4l2_ext_control c; | |
161 | + struct v4l2_ext_control c = { control->id }; | |
162 | int ret; | |
163 | ||
164 | if (ctrl == NULL || !ctrl->is_int) | |
165 | @@ -3252,7 +3260,7 @@ int __v4l2_ctrl_s_ctrl(struct v4l2_ctrl | |
166 | /* It's a driver bug if this happens. */ | |
167 | WARN_ON(!ctrl->is_int); | |
168 | ctrl->val = val; | |
169 | - return set_ctrl(NULL, ctrl, NULL, 0); | |
170 | + return set_ctrl(NULL, ctrl, 0); | |
171 | } | |
172 | EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl); | |
173 | ||
174 | @@ -3263,7 +3271,7 @@ int __v4l2_ctrl_s_ctrl_int64(struct v4l2 | |
175 | /* It's a driver bug if this happens. */ | |
176 | WARN_ON(ctrl->is_ptr || ctrl->type != V4L2_CTRL_TYPE_INTEGER64); | |
177 | *ctrl->p_new.p_s64 = val; | |
178 | - return set_ctrl(NULL, ctrl, NULL, 0); | |
179 | + return set_ctrl(NULL, ctrl, 0); | |
180 | } | |
181 | EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_int64); | |
182 | ||
183 | @@ -3274,7 +3282,7 @@ int __v4l2_ctrl_s_ctrl_string(struct v4l | |
184 | /* It's a driver bug if this happens. */ | |
185 | WARN_ON(ctrl->type != V4L2_CTRL_TYPE_STRING); | |
186 | strlcpy(ctrl->p_new.p_char, s, ctrl->maximum + 1); | |
187 | - return set_ctrl(NULL, ctrl, NULL, 0); | |
188 | + return set_ctrl(NULL, ctrl, 0); | |
189 | } | |
190 | EXPORT_SYMBOL(__v4l2_ctrl_s_ctrl_string); | |
191 | ||
192 | @@ -3297,8 +3305,8 @@ EXPORT_SYMBOL(v4l2_ctrl_notify); | |
193 | int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl, | |
194 | s64 min, s64 max, u64 step, s64 def) | |
195 | { | |
196 | + bool changed; | |
197 | int ret; | |
198 | - struct v4l2_ext_control c; | |
199 | ||
200 | lockdep_assert_held(ctrl->handler->lock); | |
201 | ||
202 | @@ -3325,11 +3333,20 @@ int __v4l2_ctrl_modify_range(struct v4l2 | |
203 | ctrl->maximum = max; | |
204 | ctrl->step = step; | |
205 | ctrl->default_value = def; | |
206 | - c.value = *ctrl->p_cur.p_s32; | |
207 | - if (validate_new(ctrl, &c)) | |
208 | - c.value = def; | |
209 | - if (c.value != *ctrl->p_cur.p_s32) | |
210 | - ret = set_ctrl(NULL, ctrl, &c, V4L2_EVENT_CTRL_CH_RANGE); | |
211 | + cur_to_new(ctrl); | |
212 | + if (validate_new(ctrl, ctrl->p_new)) { | |
213 | + if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64) | |
214 | + *ctrl->p_new.p_s64 = def; | |
215 | + else | |
216 | + *ctrl->p_new.p_s32 = def; | |
217 | + } | |
218 | + | |
219 | + if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64) | |
220 | + changed = *ctrl->p_new.p_s64 != *ctrl->p_cur.p_s64; | |
221 | + else | |
222 | + changed = *ctrl->p_new.p_s32 != *ctrl->p_cur.p_s32; | |
223 | + if (changed) | |
224 | + ret = set_ctrl(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE); | |
225 | else | |
226 | send_event(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE); | |
227 | return ret; |