* [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
@ 2012-11-21 4:29 Chen Gang
2012-12-03 8:56 ` Chen Gang
2012-12-04 3:46 ` Chas Williams (CONTRACTOR)
0 siblings, 2 replies; 16+ messages in thread
From: Chen Gang @ 2012-11-21 4:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Hello David Miller:
in net/atm/atm_sysfs.c:
suggest to check the write length whether larger than a page.
the length of parameter buf is one page size (reference: fill_read_buffer at fs/sysfs/file.c)
and the count of atm adresses are not limited (reference: atm_dev_ioctl -> atm_add_addr)
thanks.
gchen.
34 static ssize_t show_atmaddress(struct device *cdev,
35 struct device_attribute *attr, char *buf)
36 {
37 unsigned long flags;
38 char *pos = buf;
39 struct atm_dev *adev = to_atm_dev(cdev);
40 struct atm_dev_addr *aaddr;
41 int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
42 int i, j;
43
44 spin_lock_irqsave(&adev->lock, flags);
45 list_for_each_entry(aaddr, &adev->local, entry) {
46 for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
47 if (j == *fmt) {
48 pos += sprintf(pos, ".");
49 ++fmt;
50 j = 0;
51 }
52 pos += sprintf(pos, "%02x",
53 aaddr->addr.sas_addr.prv[i]);
54 }
55 pos += sprintf(pos, "\n");
56 }
57 spin_unlock_irqrestore(&adev->lock, flags);
58
59 return pos - buf;
60 }
61
in net/atm/addr.c
67 int atm_add_addr(struct atm_dev *dev, const struct sockaddr_atmsvc *addr,
68 enum atm_addr_type_t atype)
69 {
70 unsigned long flags;
71 struct atm_dev_addr *this;
72 struct list_head *head;
73 int error;
74
75 error = check_addr(addr);
76 if (error)
77 return error;
78 spin_lock_irqsave(&dev->lock, flags);
79 if (atype == ATM_ADDR_LECS)
80 head = &dev->lecs;
81 else
82 head = &dev->local;
83 list_for_each_entry(this, head, entry) {
84 if (identical(&this->addr, addr)) {
85 spin_unlock_irqrestore(&dev->lock, flags);
86 return -EEXIST;
87 }
88 }
89 this = kmalloc(sizeof(struct atm_dev_addr), GFP_ATOMIC);
90 if (!this) {
91 spin_unlock_irqrestore(&dev->lock, flags);
92 return -ENOMEM;
93 }
94 this->addr = *addr;
95 list_add(&this->entry, head);
96 spin_unlock_irqrestore(&dev->lock, flags);
97 if (head == &dev->local)
98 notify_sigd(dev);
99 return 0;
100 }
101
in net/atm/resources.c
195 int atm_dev_ioctl(unsigned int cmd, void __user *arg, int compat)
196 {
197 void __user *buf;
198 int error, len, number, size = 0;
199 struct atm_dev *dev;
200 struct list_head *p;
201 int *tmp_buf, *tmp_p;
202 int __user *sioc_len;
203 int __user *iobuf_len;
204
205 #ifndef CONFIG_COMPAT
206 compat = 0; /* Just so the compiler _knows_ */
207 #endif
208
209 switch (cmd) {
210 case ATM_GETNAMES:
211 if (compat) {
212 #ifdef CONFIG_COMPAT
213 struct compat_atm_iobuf __user *ciobuf = arg;
214 compat_uptr_t cbuf;
215 iobuf_len = &ciobuf->length;
216 if (get_user(cbuf, &ciobuf->buffer))
217 return -EFAULT;
218 buf = compat_ptr(cbuf);
219 #endif
220 } else {
221 struct atm_iobuf __user *iobuf = arg;
222 iobuf_len = &iobuf->length;
223 if (get_user(buf, &iobuf->buffer))
224 return -EFAULT;
225 }
226 if (get_user(len, iobuf_len))
227 return -EFAULT;
228 mutex_lock(&atm_dev_mutex);
229 list_for_each(p, &atm_devs)
230 size += sizeof(int);
231 if (size > len) {
232 mutex_unlock(&atm_dev_mutex);
233 return -E2BIG;
234 }
235 tmp_buf = kmalloc(size, GFP_ATOMIC);
236 if (!tmp_buf) {
237 mutex_unlock(&atm_dev_mutex);
238 return -ENOMEM;
239 }
240 tmp_p = tmp_buf;
241 list_for_each(p, &atm_devs) {
242 dev = list_entry(p, struct atm_dev, dev_list);
243 *tmp_p++ = dev->number;
244 }
245 mutex_unlock(&atm_dev_mutex);
246 error = ((copy_to_user(buf, tmp_buf, size)) ||
247 put_user(size, iobuf_len))
248 ? -EFAULT : 0;
249 kfree(tmp_buf);
250 return error;
251 default:
252 break;
253 }
254
255 if (compat) {
256 #ifdef CONFIG_COMPAT
257 struct compat_atmif_sioc __user *csioc = arg;
258 compat_uptr_t carg;
259
260 sioc_len = &csioc->length;
261 if (get_user(carg, &csioc->arg))
262 return -EFAULT;
263 buf = compat_ptr(carg);
264
265 if (get_user(len, &csioc->length))
266 return -EFAULT;
267 if (get_user(number, &csioc->number))
268 return -EFAULT;
269 #endif
270 } else {
271 struct atmif_sioc __user *sioc = arg;
272
273 sioc_len = &sioc->length;
274 if (get_user(buf, &sioc->arg))
275 return -EFAULT;
276 if (get_user(len, &sioc->length))
277 return -EFAULT;
278 if (get_user(number, &sioc->number))
279 return -EFAULT;
280 }
281
282 dev = try_then_request_module(atm_dev_lookup(number), "atm-device-%d",
283 number);
284 if (!dev)
285 return -ENODEV;
286
287 switch (cmd) {
288 case ATM_GETTYPE:
289 size = strlen(dev->type) + 1;
290 if (copy_to_user(buf, dev->type, size)) {
291 error = -EFAULT;
292 goto done;
293 }
294 break;
295 case ATM_GETESI:
296 size = ESI_LEN;
297 if (copy_to_user(buf, dev->esi, size)) {
298 error = -EFAULT;
299 goto done;
300 }
301 break;
302 case ATM_SETESI:
303 {
304 int i;
305
306 for (i = 0; i < ESI_LEN; i++)
307 if (dev->esi[i]) {
308 error = -EEXIST;
309 goto done;
310 }
311 }
312 /* fall through */
313 case ATM_SETESIF:
314 {
315 unsigned char esi[ESI_LEN];
316
317 if (!capable(CAP_NET_ADMIN)) {
318 error = -EPERM;
319 goto done;
320 }
321 if (copy_from_user(esi, buf, ESI_LEN)) {
322 error = -EFAULT;
323 goto done;
324 }
325 memcpy(dev->esi, esi, ESI_LEN);
326 error = ESI_LEN;
327 goto done;
328 }
329 case ATM_GETSTATZ:
330 if (!capable(CAP_NET_ADMIN)) {
331 error = -EPERM;
332 goto done;
333 }
334 /* fall through */
335 case ATM_GETSTAT:
336 size = sizeof(struct atm_dev_stats);
337 error = fetch_stats(dev, buf, cmd == ATM_GETSTATZ);
338 if (error)
339 goto done;
340 break;
341 case ATM_GETCIRANGE:
342 size = sizeof(struct atm_cirange);
343 if (copy_to_user(buf, &dev->ci_range, size)) {
344 error = -EFAULT;
345 goto done;
346 }
347 break;
348 case ATM_GETLINKRATE:
349 size = sizeof(int);
350 if (copy_to_user(buf, &dev->link_rate, size)) {
351 error = -EFAULT;
352 goto done;
353 }
354 break;
355 case ATM_RSTADDR:
356 if (!capable(CAP_NET_ADMIN)) {
357 error = -EPERM;
358 goto done;
359 }
360 atm_reset_addr(dev, ATM_ADDR_LOCAL);
361 break;
362 case ATM_ADDADDR:
363 case ATM_DELADDR:
364 case ATM_ADDLECSADDR:
365 case ATM_DELLECSADDR:
366 {
367 struct sockaddr_atmsvc addr;
368
369 if (!capable(CAP_NET_ADMIN)) {
370 error = -EPERM;
371 goto done;
372 }
373
374 if (copy_from_user(&addr, buf, sizeof(addr))) {
375 error = -EFAULT;
376 goto done;
377 }
378 if (cmd == ATM_ADDADDR || cmd == ATM_ADDLECSADDR)
379 error = atm_add_addr(dev, &addr,
380 (cmd == ATM_ADDADDR ?
381 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
382 else
383 error = atm_del_addr(dev, &addr,
384 (cmd == ATM_DELADDR ?
385 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
386 goto done;
387 }
... ...
... ...
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-11-21 4:29 [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page Chen Gang
@ 2012-12-03 8:56 ` Chen Gang
2012-12-03 15:48 ` chas williams - CONTRACTOR
2012-12-04 3:46 ` Chas Williams (CONTRACTOR)
1 sibling, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-03 8:56 UTC (permalink / raw)
To: David.Woodhouse, David Miller, krzysiek, Joe Perches, edumazet,
netdev
Hello Maintainers:
was this suggestion replied ? (it seems not).
and please help to check whether this suggestion is valid.
thanks.
gchen.
于 2012年11月21日 12:29, Chen Gang 写道:
> Hello David Miller:
>
> in net/atm/atm_sysfs.c:
> suggest to check the write length whether larger than a page.
> the length of parameter buf is one page size (reference: fill_read_buffer at fs/sysfs/file.c)
> and the count of atm adresses are not limited (reference: atm_dev_ioctl -> atm_add_addr)
>
> thanks.
>
> gchen.
>
> 34 static ssize_t show_atmaddress(struct device *cdev,
> 35 struct device_attribute *attr, char *buf)
> 36 {
> 37 unsigned long flags;
> 38 char *pos = buf;
> 39 struct atm_dev *adev = to_atm_dev(cdev);
> 40 struct atm_dev_addr *aaddr;
> 41 int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
> 42 int i, j;
> 43
> 44 spin_lock_irqsave(&adev->lock, flags);
> 45 list_for_each_entry(aaddr, &adev->local, entry) {
> 46 for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
> 47 if (j == *fmt) {
> 48 pos += sprintf(pos, ".");
> 49 ++fmt;
> 50 j = 0;
> 51 }
> 52 pos += sprintf(pos, "%02x",
> 53 aaddr->addr.sas_addr.prv[i]);
> 54 }
> 55 pos += sprintf(pos, "\n");
> 56 }
> 57 spin_unlock_irqrestore(&adev->lock, flags);
> 58
> 59 return pos - buf;
> 60 }
> 61
>
>
>
> in net/atm/addr.c
>
> 67 int atm_add_addr(struct atm_dev *dev, const struct sockaddr_atmsvc *addr,
> 68 enum atm_addr_type_t atype)
> 69 {
> 70 unsigned long flags;
> 71 struct atm_dev_addr *this;
> 72 struct list_head *head;
> 73 int error;
> 74
> 75 error = check_addr(addr);
> 76 if (error)
> 77 return error;
> 78 spin_lock_irqsave(&dev->lock, flags);
> 79 if (atype == ATM_ADDR_LECS)
> 80 head = &dev->lecs;
> 81 else
> 82 head = &dev->local;
> 83 list_for_each_entry(this, head, entry) {
> 84 if (identical(&this->addr, addr)) {
> 85 spin_unlock_irqrestore(&dev->lock, flags);
> 86 return -EEXIST;
> 87 }
> 88 }
> 89 this = kmalloc(sizeof(struct atm_dev_addr), GFP_ATOMIC);
> 90 if (!this) {
> 91 spin_unlock_irqrestore(&dev->lock, flags);
> 92 return -ENOMEM;
> 93 }
> 94 this->addr = *addr;
> 95 list_add(&this->entry, head);
> 96 spin_unlock_irqrestore(&dev->lock, flags);
> 97 if (head == &dev->local)
> 98 notify_sigd(dev);
> 99 return 0;
> 100 }
> 101
>
>
> in net/atm/resources.c
>
> 195 int atm_dev_ioctl(unsigned int cmd, void __user *arg, int compat)
> 196 {
> 197 void __user *buf;
> 198 int error, len, number, size = 0;
> 199 struct atm_dev *dev;
> 200 struct list_head *p;
> 201 int *tmp_buf, *tmp_p;
> 202 int __user *sioc_len;
> 203 int __user *iobuf_len;
> 204
> 205 #ifndef CONFIG_COMPAT
> 206 compat = 0; /* Just so the compiler _knows_ */
> 207 #endif
> 208
> 209 switch (cmd) {
> 210 case ATM_GETNAMES:
> 211 if (compat) {
> 212 #ifdef CONFIG_COMPAT
> 213 struct compat_atm_iobuf __user *ciobuf = arg;
> 214 compat_uptr_t cbuf;
> 215 iobuf_len = &ciobuf->length;
> 216 if (get_user(cbuf, &ciobuf->buffer))
> 217 return -EFAULT;
> 218 buf = compat_ptr(cbuf);
> 219 #endif
> 220 } else {
> 221 struct atm_iobuf __user *iobuf = arg;
> 222 iobuf_len = &iobuf->length;
> 223 if (get_user(buf, &iobuf->buffer))
> 224 return -EFAULT;
> 225 }
> 226 if (get_user(len, iobuf_len))
> 227 return -EFAULT;
> 228 mutex_lock(&atm_dev_mutex);
> 229 list_for_each(p, &atm_devs)
> 230 size += sizeof(int);
> 231 if (size > len) {
> 232 mutex_unlock(&atm_dev_mutex);
> 233 return -E2BIG;
> 234 }
> 235 tmp_buf = kmalloc(size, GFP_ATOMIC);
> 236 if (!tmp_buf) {
> 237 mutex_unlock(&atm_dev_mutex);
> 238 return -ENOMEM;
> 239 }
> 240 tmp_p = tmp_buf;
> 241 list_for_each(p, &atm_devs) {
> 242 dev = list_entry(p, struct atm_dev, dev_list);
> 243 *tmp_p++ = dev->number;
> 244 }
> 245 mutex_unlock(&atm_dev_mutex);
> 246 error = ((copy_to_user(buf, tmp_buf, size)) ||
> 247 put_user(size, iobuf_len))
> 248 ? -EFAULT : 0;
> 249 kfree(tmp_buf);
> 250 return error;
> 251 default:
> 252 break;
> 253 }
> 254
> 255 if (compat) {
> 256 #ifdef CONFIG_COMPAT
> 257 struct compat_atmif_sioc __user *csioc = arg;
> 258 compat_uptr_t carg;
> 259
> 260 sioc_len = &csioc->length;
> 261 if (get_user(carg, &csioc->arg))
> 262 return -EFAULT;
> 263 buf = compat_ptr(carg);
> 264
> 265 if (get_user(len, &csioc->length))
> 266 return -EFAULT;
> 267 if (get_user(number, &csioc->number))
> 268 return -EFAULT;
> 269 #endif
> 270 } else {
> 271 struct atmif_sioc __user *sioc = arg;
> 272
> 273 sioc_len = &sioc->length;
> 274 if (get_user(buf, &sioc->arg))
> 275 return -EFAULT;
> 276 if (get_user(len, &sioc->length))
> 277 return -EFAULT;
> 278 if (get_user(number, &sioc->number))
> 279 return -EFAULT;
> 280 }
> 281
> 282 dev = try_then_request_module(atm_dev_lookup(number), "atm-device-%d",
> 283 number);
> 284 if (!dev)
> 285 return -ENODEV;
> 286
> 287 switch (cmd) {
> 288 case ATM_GETTYPE:
> 289 size = strlen(dev->type) + 1;
> 290 if (copy_to_user(buf, dev->type, size)) {
> 291 error = -EFAULT;
> 292 goto done;
> 293 }
> 294 break;
> 295 case ATM_GETESI:
> 296 size = ESI_LEN;
> 297 if (copy_to_user(buf, dev->esi, size)) {
> 298 error = -EFAULT;
> 299 goto done;
> 300 }
> 301 break;
> 302 case ATM_SETESI:
> 303 {
> 304 int i;
> 305
> 306 for (i = 0; i < ESI_LEN; i++)
> 307 if (dev->esi[i]) {
> 308 error = -EEXIST;
> 309 goto done;
> 310 }
> 311 }
> 312 /* fall through */
> 313 case ATM_SETESIF:
> 314 {
> 315 unsigned char esi[ESI_LEN];
> 316
> 317 if (!capable(CAP_NET_ADMIN)) {
> 318 error = -EPERM;
> 319 goto done;
> 320 }
> 321 if (copy_from_user(esi, buf, ESI_LEN)) {
> 322 error = -EFAULT;
> 323 goto done;
> 324 }
> 325 memcpy(dev->esi, esi, ESI_LEN);
> 326 error = ESI_LEN;
> 327 goto done;
> 328 }
> 329 case ATM_GETSTATZ:
> 330 if (!capable(CAP_NET_ADMIN)) {
> 331 error = -EPERM;
> 332 goto done;
> 333 }
> 334 /* fall through */
> 335 case ATM_GETSTAT:
> 336 size = sizeof(struct atm_dev_stats);
> 337 error = fetch_stats(dev, buf, cmd == ATM_GETSTATZ);
> 338 if (error)
> 339 goto done;
> 340 break;
> 341 case ATM_GETCIRANGE:
> 342 size = sizeof(struct atm_cirange);
> 343 if (copy_to_user(buf, &dev->ci_range, size)) {
> 344 error = -EFAULT;
> 345 goto done;
> 346 }
> 347 break;
> 348 case ATM_GETLINKRATE:
> 349 size = sizeof(int);
> 350 if (copy_to_user(buf, &dev->link_rate, size)) {
> 351 error = -EFAULT;
> 352 goto done;
> 353 }
> 354 break;
> 355 case ATM_RSTADDR:
> 356 if (!capable(CAP_NET_ADMIN)) {
> 357 error = -EPERM;
> 358 goto done;
> 359 }
> 360 atm_reset_addr(dev, ATM_ADDR_LOCAL);
> 361 break;
> 362 case ATM_ADDADDR:
> 363 case ATM_DELADDR:
> 364 case ATM_ADDLECSADDR:
> 365 case ATM_DELLECSADDR:
> 366 {
> 367 struct sockaddr_atmsvc addr;
> 368
> 369 if (!capable(CAP_NET_ADMIN)) {
> 370 error = -EPERM;
> 371 goto done;
> 372 }
> 373
> 374 if (copy_from_user(&addr, buf, sizeof(addr))) {
> 375 error = -EFAULT;
> 376 goto done;
> 377 }
> 378 if (cmd == ATM_ADDADDR || cmd == ATM_ADDLECSADDR)
> 379 error = atm_add_addr(dev, &addr,
> 380 (cmd == ATM_ADDADDR ?
> 381 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
> 382 else
> 383 error = atm_del_addr(dev, &addr,
> 384 (cmd == ATM_DELADDR ?
> 385 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
> 386 goto done;
> 387 }
> ... ...
> ... ...
>
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-03 8:56 ` Chen Gang
@ 2012-12-03 15:48 ` chas williams - CONTRACTOR
2012-12-05 1:28 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: chas williams - CONTRACTOR @ 2012-12-03 15:48 UTC (permalink / raw)
To: Chen Gang
Cc: David.Woodhouse, David Miller, krzysiek, Joe Perches, edumazet,
netdev
yes this seems like it should be done. maybe this week i will try to
put something together unless you already have a patch somewhere.
On Mon, 03 Dec 2012 16:56:56 +0800
Chen Gang <gang.chen@asianux.com> wrote:
> Hello Maintainers:
>
> was this suggestion replied ? (it seems not).
>
> and please help to check whether this suggestion is valid.
>
> thanks.
>
> gchen.
>
>
> 于 2012年11月21日 12:29, Chen Gang 写道:
> > Hello David Miller:
> >
> > in net/atm/atm_sysfs.c:
> > suggest to check the write length whether larger than a page.
> > the length of parameter buf is one page size (reference: fill_read_buffer at fs/sysfs/file.c)
> > and the count of atm adresses are not limited (reference: atm_dev_ioctl -> atm_add_addr)
> >
> > thanks.
> >
> > gchen.
> >
> > 34 static ssize_t show_atmaddress(struct device *cdev,
> > 35 struct device_attribute *attr, char *buf)
> > 36 {
> > 37 unsigned long flags;
> > 38 char *pos = buf;
> > 39 struct atm_dev *adev = to_atm_dev(cdev);
> > 40 struct atm_dev_addr *aaddr;
> > 41 int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
> > 42 int i, j;
> > 43
> > 44 spin_lock_irqsave(&adev->lock, flags);
> > 45 list_for_each_entry(aaddr, &adev->local, entry) {
> > 46 for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
> > 47 if (j == *fmt) {
> > 48 pos += sprintf(pos, ".");
> > 49 ++fmt;
> > 50 j = 0;
> > 51 }
> > 52 pos += sprintf(pos, "%02x",
> > 53 aaddr->addr.sas_addr.prv[i]);
> > 54 }
> > 55 pos += sprintf(pos, "\n");
> > 56 }
> > 57 spin_unlock_irqrestore(&adev->lock, flags);
> > 58
> > 59 return pos - buf;
> > 60 }
> > 61
> >
> >
> >
> > in net/atm/addr.c
> >
> > 67 int atm_add_addr(struct atm_dev *dev, const struct sockaddr_atmsvc *addr,
> > 68 enum atm_addr_type_t atype)
> > 69 {
> > 70 unsigned long flags;
> > 71 struct atm_dev_addr *this;
> > 72 struct list_head *head;
> > 73 int error;
> > 74
> > 75 error = check_addr(addr);
> > 76 if (error)
> > 77 return error;
> > 78 spin_lock_irqsave(&dev->lock, flags);
> > 79 if (atype == ATM_ADDR_LECS)
> > 80 head = &dev->lecs;
> > 81 else
> > 82 head = &dev->local;
> > 83 list_for_each_entry(this, head, entry) {
> > 84 if (identical(&this->addr, addr)) {
> > 85 spin_unlock_irqrestore(&dev->lock, flags);
> > 86 return -EEXIST;
> > 87 }
> > 88 }
> > 89 this = kmalloc(sizeof(struct atm_dev_addr), GFP_ATOMIC);
> > 90 if (!this) {
> > 91 spin_unlock_irqrestore(&dev->lock, flags);
> > 92 return -ENOMEM;
> > 93 }
> > 94 this->addr = *addr;
> > 95 list_add(&this->entry, head);
> > 96 spin_unlock_irqrestore(&dev->lock, flags);
> > 97 if (head == &dev->local)
> > 98 notify_sigd(dev);
> > 99 return 0;
> > 100 }
> > 101
> >
> >
> > in net/atm/resources.c
> >
> > 195 int atm_dev_ioctl(unsigned int cmd, void __user *arg, int compat)
> > 196 {
> > 197 void __user *buf;
> > 198 int error, len, number, size = 0;
> > 199 struct atm_dev *dev;
> > 200 struct list_head *p;
> > 201 int *tmp_buf, *tmp_p;
> > 202 int __user *sioc_len;
> > 203 int __user *iobuf_len;
> > 204
> > 205 #ifndef CONFIG_COMPAT
> > 206 compat = 0; /* Just so the compiler _knows_ */
> > 207 #endif
> > 208
> > 209 switch (cmd) {
> > 210 case ATM_GETNAMES:
> > 211 if (compat) {
> > 212 #ifdef CONFIG_COMPAT
> > 213 struct compat_atm_iobuf __user *ciobuf = arg;
> > 214 compat_uptr_t cbuf;
> > 215 iobuf_len = &ciobuf->length;
> > 216 if (get_user(cbuf, &ciobuf->buffer))
> > 217 return -EFAULT;
> > 218 buf = compat_ptr(cbuf);
> > 219 #endif
> > 220 } else {
> > 221 struct atm_iobuf __user *iobuf = arg;
> > 222 iobuf_len = &iobuf->length;
> > 223 if (get_user(buf, &iobuf->buffer))
> > 224 return -EFAULT;
> > 225 }
> > 226 if (get_user(len, iobuf_len))
> > 227 return -EFAULT;
> > 228 mutex_lock(&atm_dev_mutex);
> > 229 list_for_each(p, &atm_devs)
> > 230 size += sizeof(int);
> > 231 if (size > len) {
> > 232 mutex_unlock(&atm_dev_mutex);
> > 233 return -E2BIG;
> > 234 }
> > 235 tmp_buf = kmalloc(size, GFP_ATOMIC);
> > 236 if (!tmp_buf) {
> > 237 mutex_unlock(&atm_dev_mutex);
> > 238 return -ENOMEM;
> > 239 }
> > 240 tmp_p = tmp_buf;
> > 241 list_for_each(p, &atm_devs) {
> > 242 dev = list_entry(p, struct atm_dev, dev_list);
> > 243 *tmp_p++ = dev->number;
> > 244 }
> > 245 mutex_unlock(&atm_dev_mutex);
> > 246 error = ((copy_to_user(buf, tmp_buf, size)) ||
> > 247 put_user(size, iobuf_len))
> > 248 ? -EFAULT : 0;
> > 249 kfree(tmp_buf);
> > 250 return error;
> > 251 default:
> > 252 break;
> > 253 }
> > 254
> > 255 if (compat) {
> > 256 #ifdef CONFIG_COMPAT
> > 257 struct compat_atmif_sioc __user *csioc = arg;
> > 258 compat_uptr_t carg;
> > 259
> > 260 sioc_len = &csioc->length;
> > 261 if (get_user(carg, &csioc->arg))
> > 262 return -EFAULT;
> > 263 buf = compat_ptr(carg);
> > 264
> > 265 if (get_user(len, &csioc->length))
> > 266 return -EFAULT;
> > 267 if (get_user(number, &csioc->number))
> > 268 return -EFAULT;
> > 269 #endif
> > 270 } else {
> > 271 struct atmif_sioc __user *sioc = arg;
> > 272
> > 273 sioc_len = &sioc->length;
> > 274 if (get_user(buf, &sioc->arg))
> > 275 return -EFAULT;
> > 276 if (get_user(len, &sioc->length))
> > 277 return -EFAULT;
> > 278 if (get_user(number, &sioc->number))
> > 279 return -EFAULT;
> > 280 }
> > 281
> > 282 dev = try_then_request_module(atm_dev_lookup(number), "atm-device-%d",
> > 283 number);
> > 284 if (!dev)
> > 285 return -ENODEV;
> > 286
> > 287 switch (cmd) {
> > 288 case ATM_GETTYPE:
> > 289 size = strlen(dev->type) + 1;
> > 290 if (copy_to_user(buf, dev->type, size)) {
> > 291 error = -EFAULT;
> > 292 goto done;
> > 293 }
> > 294 break;
> > 295 case ATM_GETESI:
> > 296 size = ESI_LEN;
> > 297 if (copy_to_user(buf, dev->esi, size)) {
> > 298 error = -EFAULT;
> > 299 goto done;
> > 300 }
> > 301 break;
> > 302 case ATM_SETESI:
> > 303 {
> > 304 int i;
> > 305
> > 306 for (i = 0; i < ESI_LEN; i++)
> > 307 if (dev->esi[i]) {
> > 308 error = -EEXIST;
> > 309 goto done;
> > 310 }
> > 311 }
> > 312 /* fall through */
> > 313 case ATM_SETESIF:
> > 314 {
> > 315 unsigned char esi[ESI_LEN];
> > 316
> > 317 if (!capable(CAP_NET_ADMIN)) {
> > 318 error = -EPERM;
> > 319 goto done;
> > 320 }
> > 321 if (copy_from_user(esi, buf, ESI_LEN)) {
> > 322 error = -EFAULT;
> > 323 goto done;
> > 324 }
> > 325 memcpy(dev->esi, esi, ESI_LEN);
> > 326 error = ESI_LEN;
> > 327 goto done;
> > 328 }
> > 329 case ATM_GETSTATZ:
> > 330 if (!capable(CAP_NET_ADMIN)) {
> > 331 error = -EPERM;
> > 332 goto done;
> > 333 }
> > 334 /* fall through */
> > 335 case ATM_GETSTAT:
> > 336 size = sizeof(struct atm_dev_stats);
> > 337 error = fetch_stats(dev, buf, cmd == ATM_GETSTATZ);
> > 338 if (error)
> > 339 goto done;
> > 340 break;
> > 341 case ATM_GETCIRANGE:
> > 342 size = sizeof(struct atm_cirange);
> > 343 if (copy_to_user(buf, &dev->ci_range, size)) {
> > 344 error = -EFAULT;
> > 345 goto done;
> > 346 }
> > 347 break;
> > 348 case ATM_GETLINKRATE:
> > 349 size = sizeof(int);
> > 350 if (copy_to_user(buf, &dev->link_rate, size)) {
> > 351 error = -EFAULT;
> > 352 goto done;
> > 353 }
> > 354 break;
> > 355 case ATM_RSTADDR:
> > 356 if (!capable(CAP_NET_ADMIN)) {
> > 357 error = -EPERM;
> > 358 goto done;
> > 359 }
> > 360 atm_reset_addr(dev, ATM_ADDR_LOCAL);
> > 361 break;
> > 362 case ATM_ADDADDR:
> > 363 case ATM_DELADDR:
> > 364 case ATM_ADDLECSADDR:
> > 365 case ATM_DELLECSADDR:
> > 366 {
> > 367 struct sockaddr_atmsvc addr;
> > 368
> > 369 if (!capable(CAP_NET_ADMIN)) {
> > 370 error = -EPERM;
> > 371 goto done;
> > 372 }
> > 373
> > 374 if (copy_from_user(&addr, buf, sizeof(addr))) {
> > 375 error = -EFAULT;
> > 376 goto done;
> > 377 }
> > 378 if (cmd == ATM_ADDADDR || cmd == ATM_ADDLECSADDR)
> > 379 error = atm_add_addr(dev, &addr,
> > 380 (cmd == ATM_ADDADDR ?
> > 381 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
> > 382 else
> > 383 error = atm_del_addr(dev, &addr,
> > 384 (cmd == ATM_DELADDR ?
> > 385 ATM_ADDR_LOCAL : ATM_ADDR_LECS));
> > 386 goto done;
> > 387 }
> > ... ...
> > ... ...
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-11-21 4:29 [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page Chen Gang
2012-12-03 8:56 ` Chen Gang
@ 2012-12-04 3:46 ` Chas Williams (CONTRACTOR)
2012-12-05 1:26 ` Chen Gang
1 sibling, 1 reply; 16+ messages in thread
From: Chas Williams (CONTRACTOR) @ 2012-12-04 3:46 UTC (permalink / raw)
To: Chen Gang; +Cc: David Miller, netdev
In message <50AC58BC.1020004@asianux.com>,Chen Gang writes:
>in net/atm/atm_sysfs.c:
> suggest to check the write length whether larger than a page.
> the length of parameter buf is one page size (reference: fill_read_buffer at fs/sysfs/file.c)
> and the count of atm adresses are not limited (reference: atm_dev_ioctl -> atm_add_addr)
>
> thanks.
>
>gchen.
how about this as a possible fix?
atm: use scnprintf() instead of sprintf()
As reported by Chen Gang <gang.chen@asianux.com>, we should ensure there
is enough space when formatting the sysfs buffers.
Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>
---
net/atm/atm_sysfs.c | 40 +++++++++++++++-------------------------
1 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
index f49da58..350bf62 100644
--- a/net/atm/atm_sysfs.c
+++ b/net/atm/atm_sysfs.c
@@ -14,49 +14,45 @@ static ssize_t show_type(struct device *cdev,
struct device_attribute *attr, char *buf)
{
struct atm_dev *adev = to_atm_dev(cdev);
- return sprintf(buf, "%s\n", adev->type);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", adev->type);
}
static ssize_t show_address(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- int i;
-
- for (i = 0; i < (ESI_LEN - 1); i++)
- pos += sprintf(pos, "%02x:", adev->esi[i]);
- pos += sprintf(pos, "%02x\n", adev->esi[i]);
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
}
static ssize_t show_atmaddress(struct device *cdev,
struct device_attribute *attr, char *buf)
{
unsigned long flags;
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
struct atm_dev_addr *aaddr;
int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
- int i, j;
+ int i, j, count = 0;
spin_lock_irqsave(&adev->lock, flags);
list_for_each_entry(aaddr, &adev->local, entry) {
for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
if (j == *fmt) {
- pos += sprintf(pos, ".");
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count, ".");
++fmt;
j = 0;
}
- pos += sprintf(pos, "%02x",
- aaddr->addr.sas_addr.prv[i]);
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count, "%02x",
+ aaddr->addr.sas_addr.prv[i]);
}
- pos += sprintf(pos, "\n");
+ count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
}
spin_unlock_irqrestore(&adev->lock, flags);
- return pos - buf;
+ return count;
}
static ssize_t show_atmindex(struct device *cdev,
@@ -64,25 +60,21 @@ static ssize_t show_atmindex(struct device *cdev,
{
struct atm_dev *adev = to_atm_dev(cdev);
- return sprintf(buf, "%d\n", adev->number);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", adev->number);
}
static ssize_t show_carrier(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- pos += sprintf(pos, "%d\n",
- adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
-
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
+ adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
}
static ssize_t show_link_rate(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
int link_rate;
@@ -100,9 +92,7 @@ static ssize_t show_link_rate(struct device *cdev,
default:
link_rate = adev->link_rate * 8 * 53;
}
- pos += sprintf(pos, "%d\n", link_rate);
-
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%d\n", link_rate);
}
static DEVICE_ATTR(address, S_IRUGO, show_address, NULL);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-04 3:46 ` Chas Williams (CONTRACTOR)
@ 2012-12-05 1:26 ` Chen Gang
2012-12-05 3:57 ` Chas Williams (CONTRACTOR)
0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-05 1:26 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev
firstly, sorry for reply late (yesterday I have an annual leave for
some personal things).
and thank you for your reply, too.
于 2012年12月04日 11:46, Chas Williams (CONTRACTOR) 写道:
> static ssize_t show_address(struct device *cdev,
> struct device_attribute *attr, char *buf)
> {
> - char *pos = buf;
> struct atm_dev *adev = to_atm_dev(cdev);
> - int i;
> -
> - for (i = 0; i < (ESI_LEN - 1); i++)
> - pos += sprintf(pos, "%02x:", adev->esi[i]);
> - pos += sprintf(pos, "%02x\n", adev->esi[i]);
>
> - return pos - buf;
> + return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
> }
>
"%p" seems print a pointer, not contents of pointer (is it correct ?)
will it change the original display format to outside ?
> static ssize_t show_atmaddress(struct device *cdev,
> struct device_attribute *attr, char *buf)
> {
> unsigned long flags;
> - char *pos = buf;
> struct atm_dev *adev = to_atm_dev(cdev);
> struct atm_dev_addr *aaddr;
> int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
> - int i, j;
> + int i, j, count = 0;
>
> spin_lock_irqsave(&adev->lock, flags);
> list_for_each_entry(aaddr, &adev->local, entry) {
> for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
> if (j == *fmt) {
> - pos += sprintf(pos, ".");
> + count += scnprintf(buf + count,
> + PAGE_SIZE - count, ".");
> ++fmt;
> j = 0;
> }
> - pos += sprintf(pos, "%02x",
> - aaddr->addr.sas_addr.prv[i]);
> + count += scnprintf(buf + count,
> + PAGE_SIZE - count, "%02x",
> + aaddr->addr.sas_addr.prv[i]);
> }
> - pos += sprintf(pos, "\n");
> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
> }
> spin_unlock_irqrestore(&adev->lock, flags);
>
> - return pos - buf;
> + return count;
> }
>
need we judge whether count >= PAGE_SIZE ?
these are my suggestions, maybe not correct.
:-)
thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-03 15:48 ` chas williams - CONTRACTOR
@ 2012-12-05 1:28 ` Chen Gang
0 siblings, 0 replies; 16+ messages in thread
From: Chen Gang @ 2012-12-05 1:28 UTC (permalink / raw)
To: chas williams - CONTRACTOR
Cc: David.Woodhouse, David Miller, krzysiek, Joe Perches, edumazet,
netdev
于 2012年12月03日 23:48, chas williams - CONTRACTOR 写道:
> yes this seems like it should be done. maybe this week i will try to
> put something together unless you already have a patch somewhere.
>
thank you very much.
and also sorry for replying late (yesterday, I have an annual leave
for some personal things)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 1:26 ` Chen Gang
@ 2012-12-05 3:57 ` Chas Williams (CONTRACTOR)
2012-12-05 4:56 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: Chas Williams (CONTRACTOR) @ 2012-12-05 3:57 UTC (permalink / raw)
To: Chen Gang; +Cc: David Miller, netdev
In message <50BEA2CB.9000800@asianux.com>,Chen Gang writes:
>> - for (i = 0; i < (ESI_LEN - 1); i++)
>> - pos += sprintf(pos, "%02x:", adev->esi[i]);
>> - pos += sprintf(pos, "%02x\n", adev->esi[i]);
>>
>> - return pos - buf;
>> + return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
>> }
>>
>
> "%p" seems print a pointer, not contents of pointer (is it correct ?)
> will it change the original display format to outside ?
%pM means format this pointer as a mac address. it didnt exist when the
atm stack was originally written but can be used now to save a bit of
messy code.
>> - pos += sprintf(pos, "\n");
>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
...
> need we judge whether count >= PAGE_SIZE ?
count will eventually make PAGE_SIZE - count reach 0 at which point,
scnprintf() won't be able to write into the buffer.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 3:57 ` Chas Williams (CONTRACTOR)
@ 2012-12-05 4:56 ` Chen Gang
2012-12-05 5:40 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-05 4:56 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev
于 2012年12月05日 11:57, Chas Williams (CONTRACTOR) 写道:
> In message <50BEA2CB.9000800@asianux.com>,Chen Gang writes:
>>> - for (i = 0; i < (ESI_LEN - 1); i++)
>>> - pos += sprintf(pos, "%02x:", adev->esi[i]);
>>> - pos += sprintf(pos, "%02x\n", adev->esi[i]);
>>>
>>> - return pos - buf;
>>> + return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
>>> }
>>>
>>
>> "%p" seems print a pointer, not contents of pointer (is it correct ?)
>> will it change the original display format to outside ?
>
> %pM means format this pointer as a mac address. it didnt exist when the
> atm stack was originally written but can be used now to save a bit of
> messy code.
>
it is my fault. thank you
:-)
>>> - pos += sprintf(pos, "\n");
>>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
> ..
>> need we judge whether count >= PAGE_SIZE ?
>
> count will eventually make PAGE_SIZE - count reach 0 at which point,
> scnprintf() won't be able to write into the buffer.
I also think so.
I think, maybe it will be better to break the loop when we already
know that "count >= PAGE_SIZE" (it can save waste looping, although it
seems unlikly happen, for example, using unlikly(...) ).
By the way:
will it be better that always let "\n" at the end ?
(if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
I think what I said above are minor, if you think, for this patch, do
not need consider them, it is ok (at least for me, it is true).
:-)
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 4:56 ` Chen Gang
@ 2012-12-05 5:40 ` Chen Gang
2012-12-05 5:59 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-05 5:40 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev
于 2012年12月05日 12:56, Chen Gang 写道:
>>>> >>> - pos += sprintf(pos, "\n");
>>>> >>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
>> > ..
>>> >> need we judge whether count >= PAGE_SIZE ?
>> >
>> > count will eventually make PAGE_SIZE - count reach 0 at which point,
>> > scnprintf() won't be able to write into the buffer.
> I also think so.
>
> I think, maybe it will be better to break the loop when we already
> know that "count >= PAGE_SIZE" (it can save waste looping, although it
> seems unlikly happen, for example, using unlikly(...) ).
>
> By the way:
> will it be better that always let "\n" at the end ?
> (if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
oh, sorry ! count will never >= PAGE_SIZE.
I think let "PAGE_SIZE - 2" instead of "PAGE_SIZE" in the loop, so we
can make the room for the end of "\n".
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 5:40 ` Chen Gang
@ 2012-12-05 5:59 ` Chen Gang
2012-12-05 14:55 ` chas williams - CONTRACTOR
0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-05 5:59 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR); +Cc: David Miller, netdev
于 2012年12月05日 13:40, Chen Gang 写道:
> 于 2012年12月05日 12:56, Chen Gang 写道:
>>>>>>>> - pos += sprintf(pos, "\n");
>>>>>>>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
>>>> ..
>>>>>> need we judge whether count >= PAGE_SIZE ?
>>>>
>>>> count will eventually make PAGE_SIZE - count reach 0 at which point,
>>>> scnprintf() won't be able to write into the buffer.
>> I also think so.
>>
>> I think, maybe it will be better to break the loop when we already
>> know that "count >= PAGE_SIZE" (it can save waste looping, although it
>> seems unlikly happen, for example, using unlikly(...) ).
>>
>> By the way:
>> will it be better that always let "\n" at the end ?
>> (if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
>
> oh, sorry ! count will never >= PAGE_SIZE.
>
> I think let "PAGE_SIZE - 2" instead of "PAGE_SIZE" in the loop, so we
> can make the room for the end of "\n".
>
>
>
sorry, "PAGE_SIZE - 1" is enough, not need "PAGE_SIZE - 2".
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 5:59 ` Chen Gang
@ 2012-12-05 14:55 ` chas williams - CONTRACTOR
2012-12-06 1:15 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: chas williams - CONTRACTOR @ 2012-12-05 14:55 UTC (permalink / raw)
To: Chen Gang; +Cc: David Miller, netdev
On Wed, 05 Dec 2012 13:59:26 +0800
Chen Gang <gang.chen@asianux.com> wrote:
> 于 2012年12月05日 13:40, Chen Gang 写道:
> > 于 2012年12月05日 12:56, Chen Gang 写道:
> >>>>>>>> - pos += sprintf(pos, "\n");
> >>>>>>>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
> >>>> ..
> >>>>>> need we judge whether count >= PAGE_SIZE ?
> >>>>
> >>>> count will eventually make PAGE_SIZE - count reach 0 at which point,
> >>>> scnprintf() won't be able to write into the buffer.
> >> I also think so.
> >>
> >> I think, maybe it will be better to break the loop when we already
> >> know that "count >= PAGE_SIZE" (it can save waste looping, although it
> >> seems unlikly happen, for example, using unlikly(...) ).
it doesn't seem like optimizing for this corner case is a huge
concern. the list cannot be infinitely long.
> >>
> >> By the way:
> >> will it be better that always let "\n" at the end ?
> >> (if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
> >
> > oh, sorry ! count will never >= PAGE_SIZE.
> >
> > I think let "PAGE_SIZE - 2" instead of "PAGE_SIZE" in the loop, so we
> > can make the room for the end of "\n".
> >
> >
> >
> sorry, "PAGE_SIZE - 1" is enough, not need "PAGE_SIZE - 2".
did you mean '\0' instead of '\n'? scnprintf() considers the trailing
'\0' when formatting.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-05 14:55 ` chas williams - CONTRACTOR
@ 2012-12-06 1:15 ` Chen Gang
2012-12-06 9:05 ` Chen Gang
2012-12-06 14:08 ` chas williams - CONTRACTOR
0 siblings, 2 replies; 16+ messages in thread
From: Chen Gang @ 2012-12-06 1:15 UTC (permalink / raw)
To: chas williams - CONTRACTOR; +Cc: David Miller, netdev
于 2012年12月05日 22:55, chas williams - CONTRACTOR 写道:
> On Wed, 05 Dec 2012 13:59:26 +0800
>
> it doesn't seem like optimizing for this corner case is a huge
> concern. the list cannot be infinitely long.
>
ok.
>>>>
>>>> By the way:
>>>> will it be better that always let "\n" at the end ?
>>>> (if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
>>>
>>> oh, sorry ! count will never >= PAGE_SIZE.
>>>
>>> I think let "PAGE_SIZE - 2" instead of "PAGE_SIZE" in the loop, so we
>>> can make the room for the end of "\n".
>>>
>>>
>>>
>> sorry, "PAGE_SIZE - 1" is enough, not need "PAGE_SIZE - 2".
>
> did you mean '\0' instead of '\n'? scnprintf() considers the trailing
> '\0' when formatting.
no, originally, the end is "\n\0".
I prefer we still compatible "\n" when the contents are very large.
if count already == (PAGE_SIZE - 1), we have no chance to append "\n" to the end.
- pos += sprintf(pos, "\n");
+ count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-06 1:15 ` Chen Gang
@ 2012-12-06 9:05 ` Chen Gang
2012-12-06 14:08 ` chas williams - CONTRACTOR
1 sibling, 0 replies; 16+ messages in thread
From: Chen Gang @ 2012-12-06 9:05 UTC (permalink / raw)
To: chas williams - CONTRACTOR; +Cc: David Miller, netdev
Hi Chas Williams:
all of my original reply are my idea (or suggestions), not for issues.
if you do not need them, please help to send patch.
I have tried to check it. at least, I did not find issues (and I also
also learned from it)
hope the patch can pass reviewers checking !
Good Luck !
:-)
gchen.
于 2012年12月06日 09:15, Chen Gang 写道:
> 于 2012年12月05日 22:55, chas williams - CONTRACTOR 写道:
>> On Wed, 05 Dec 2012 13:59:26 +0800
>>
>> it doesn't seem like optimizing for this corner case is a huge
>> concern. the list cannot be infinitely long.
>>
>
> ok.
>
>
>>>>>
>>>>> By the way:
>>>>> will it be better that always let "\n" at the end ?
>>>>> (if count == PAGE_SIZE in a loop, we can not let "\n" at the end).
>>>>
>>>> oh, sorry ! count will never >= PAGE_SIZE.
>>>>
>>>> I think let "PAGE_SIZE - 2" instead of "PAGE_SIZE" in the loop, so we
>>>> can make the room for the end of "\n".
>>>>
>>>>
>>>>
>>> sorry, "PAGE_SIZE - 1" is enough, not need "PAGE_SIZE - 2".
>>
>> did you mean '\0' instead of '\n'? scnprintf() considers the trailing
>> '\0' when formatting.
>
> no, originally, the end is "\n\0".
>
> I prefer we still compatible "\n" when the contents are very large.
> if count already == (PAGE_SIZE - 1), we have no chance to append "\n" to the end.
>
> - pos += sprintf(pos, "\n");
> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
>
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-06 1:15 ` Chen Gang
2012-12-06 9:05 ` Chen Gang
@ 2012-12-06 14:08 ` chas williams - CONTRACTOR
2012-12-07 1:07 ` Chen Gang
1 sibling, 1 reply; 16+ messages in thread
From: chas williams - CONTRACTOR @ 2012-12-06 14:08 UTC (permalink / raw)
To: Chen Gang; +Cc: David Miller, netdev
On Thu, 06 Dec 2012 09:15:10 +0800
Chen Gang <gang.chen@asianux.com> wrote:
> 于 2012年12月05日 22:55, chas williams - CONTRACTOR 写道:
> > did you mean '\0' instead of '\n'? scnprintf() considers the trailing
> > '\0' when formatting.
>
> no, originally, the end is "\n\0".
>
> I prefer we still compatible "\n" when the contents are very large.
> if count already == (PAGE_SIZE - 1), we have no chance to append "\n" to the end.
>
> - pos += sprintf(pos, "\n");
> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
i would make the code a bit messy to do this for not much gain. again,
it isnt likely you would run into this in a normal situation.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-06 14:08 ` chas williams - CONTRACTOR
@ 2012-12-07 1:07 ` Chen Gang
2012-12-10 1:39 ` Chen Gang
0 siblings, 1 reply; 16+ messages in thread
From: Chen Gang @ 2012-12-07 1:07 UTC (permalink / raw)
To: chas williams - CONTRACTOR; +Cc: David Miller, netdev
于 2012年12月06日 22:08, chas williams - CONTRACTOR 写道:
> On Thu, 06 Dec 2012 09:15:10 +0800
> Chen Gang <gang.chen@asianux.com> wrote:
>
>> 于 2012年12月05日 22:55, chas williams - CONTRACTOR 写道:
>
>>> did you mean '\0' instead of '\n'? scnprintf() considers the trailing
>>> '\0' when formatting.
>>
>> no, originally, the end is "\n\0".
>>
>> I prefer we still compatible "\n" when the contents are very large.
>> if count already == (PAGE_SIZE - 1), we have no chance to append "\n" to the end.
>>
>> - pos += sprintf(pos, "\n");
>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
>
> i would make the code a bit messy to do this for not much gain. again,
> it isnt likely you would run into this in a normal situation.
surely.
thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
2012-12-07 1:07 ` Chen Gang
@ 2012-12-10 1:39 ` Chen Gang
0 siblings, 0 replies; 16+ messages in thread
From: Chen Gang @ 2012-12-10 1:39 UTC (permalink / raw)
To: chas williams - CONTRACTOR; +Cc: David Miller, netdev
Hello Chas Williams:
Excuse me:
I am a reporter (not a reviewer), and not suitable to review another's patch.
so:
I suggest you to send your patch (as your willing) to the reviewers, directly.
and need not cc to me (I am not a reviewer).
thanks.
gchen.
于 2012年12月07日 09:07, Chen Gang 写道:
> 于 2012年12月06日 22:08, chas williams - CONTRACTOR 写道:
>> On Thu, 06 Dec 2012 09:15:10 +0800
>> Chen Gang <gang.chen@asianux.com> wrote:
>>
>>> 于 2012年12月05日 22:55, chas williams - CONTRACTOR 写道:
>>
>>>> did you mean '\0' instead of '\n'? scnprintf() considers the trailing
>>>> '\0' when formatting.
>>>
>>> no, originally, the end is "\n\0".
>>>
>>> I prefer we still compatible "\n" when the contents are very large.
>>> if count already == (PAGE_SIZE - 1), we have no chance to append "\n" to the end.
>>>
>>> - pos += sprintf(pos, "\n");
>>> + count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
>>
>> i would make the code a bit messy to do this for not much gain. again,
>> it isnt likely you would run into this in a normal situation.
>
> surely.
>
> thanks.
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-12-10 1:38 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-21 4:29 [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page Chen Gang
2012-12-03 8:56 ` Chen Gang
2012-12-03 15:48 ` chas williams - CONTRACTOR
2012-12-05 1:28 ` Chen Gang
2012-12-04 3:46 ` Chas Williams (CONTRACTOR)
2012-12-05 1:26 ` Chen Gang
2012-12-05 3:57 ` Chas Williams (CONTRACTOR)
2012-12-05 4:56 ` Chen Gang
2012-12-05 5:40 ` Chen Gang
2012-12-05 5:59 ` Chen Gang
2012-12-05 14:55 ` chas williams - CONTRACTOR
2012-12-06 1:15 ` Chen Gang
2012-12-06 9:05 ` Chen Gang
2012-12-06 14:08 ` chas williams - CONTRACTOR
2012-12-07 1:07 ` Chen Gang
2012-12-10 1:39 ` Chen Gang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).