diff options
author | Alexey Dobriyan <adobriyan@gmail.com> | 2008-04-13 07:48:43 -0400 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@infradead.org> | 2008-04-24 13:09:40 -0400 |
commit | 9faa2d75822e1950b3aacc8ccbdf0cdb595e47de (patch) | |
tree | da40b056cfb87ccad6414f0dd2b37abbe019fe35 /drivers/media/video/videocodec.c | |
parent | b9bc07a006ae94d7b3dd5db873bcf10ceb749253 (diff) |
V4L/DVB (7580): Fix concurrent read from /proc/videocodecs
Observation one: ->write_proc and ->data assignments aren't needed. Removed.
Observation two: codecs lists are unprotected. Patch doesn't fix this.
Observation three:
/proc/videocodecs printout is done to temporary _global_ buffer which
is freed in between. Consequently, two users hitting this file can
screwup each other.
Steps to reproduce:
modprobe videocodec
while true; do cat /proc/videocodecs &>/dev/null; done &
while true; do cat /proc/videocodecs &>/dev/null; done &
The fix is switching to seq_files, this removes code, especially some
line-length "logic".
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Acked-by: Jan Kara <jack@suse.cz>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
Diffstat (limited to 'drivers/media/video/videocodec.c')
-rw-r--r-- | drivers/media/video/videocodec.c | 113 |
1 files changed, 19 insertions, 94 deletions
diff --git a/drivers/media/video/videocodec.c b/drivers/media/video/videocodec.c index b30b6b2a9a85..cf24956f3204 100644 --- a/drivers/media/video/videocodec.c +++ b/drivers/media/video/videocodec.c | |||
@@ -39,6 +39,7 @@ | |||
39 | 39 | ||
40 | #ifdef CONFIG_PROC_FS | 40 | #ifdef CONFIG_PROC_FS |
41 | #include <linux/proc_fs.h> | 41 | #include <linux/proc_fs.h> |
42 | #include <linux/seq_file.h> | ||
42 | #include <asm/uaccess.h> | 43 | #include <asm/uaccess.h> |
43 | #endif | 44 | #endif |
44 | 45 | ||
@@ -320,56 +321,22 @@ videocodec_unregister (const struct videocodec *codec) | |||
320 | } | 321 | } |
321 | 322 | ||
322 | #ifdef CONFIG_PROC_FS | 323 | #ifdef CONFIG_PROC_FS |
323 | /* ============ */ | 324 | static int proc_videocodecs_show(struct seq_file *m, void *v) |
324 | /* procfs stuff */ | ||
325 | /* ============ */ | ||
326 | |||
327 | static char *videocodec_buf = NULL; | ||
328 | static int videocodec_bufsize; | ||
329 | |||
330 | static int | ||
331 | videocodec_build_table (void) | ||
332 | { | 325 | { |
333 | struct codec_list *h = codeclist_top; | 326 | struct codec_list *h = codeclist_top; |
334 | struct attached_list *a; | 327 | struct attached_list *a; |
335 | int i = 0, size; | ||
336 | |||
337 | // sum up amount of slaves plus their attached masters | ||
338 | while (h) { | ||
339 | i += h->attached + 1; | ||
340 | h = h->next; | ||
341 | } | ||
342 | #define LINESIZE 100 | ||
343 | size = LINESIZE * (i + 1); | ||
344 | 328 | ||
345 | dprintk(3, "videocodec_build table: %d entries, %d bytes\n", i, | 329 | seq_printf(m, "<S>lave or attached <M>aster name type flags magic "); |
346 | size); | 330 | seq_printf(m, "(connected as)\n"); |
347 | |||
348 | kfree(videocodec_buf); | ||
349 | videocodec_buf = kmalloc(size, GFP_KERNEL); | ||
350 | |||
351 | if (!videocodec_buf) | ||
352 | return 0; | ||
353 | |||
354 | i = 0; | ||
355 | i += scnprintf(videocodec_buf + i, size - 1, | ||
356 | "<S>lave or attached <M>aster name type flags magic "); | ||
357 | i += scnprintf(videocodec_buf + i, size -i - 1, "(connected as)\n"); | ||
358 | 331 | ||
359 | h = codeclist_top; | 332 | h = codeclist_top; |
360 | while (h) { | 333 | while (h) { |
361 | if (i > (size - LINESIZE)) | 334 | seq_printf(m, "S %32s %04x %08lx %08lx (TEMPLATE)\n", |
362 | break; // security check | ||
363 | i += scnprintf(videocodec_buf + i, size -i -1, | ||
364 | "S %32s %04x %08lx %08lx (TEMPLATE)\n", | ||
365 | h->codec->name, h->codec->type, | 335 | h->codec->name, h->codec->type, |
366 | h->codec->flags, h->codec->magic); | 336 | h->codec->flags, h->codec->magic); |
367 | a = h->list; | 337 | a = h->list; |
368 | while (a) { | 338 | while (a) { |
369 | if (i > (size - LINESIZE)) | 339 | seq_printf(m, "M %32s %04x %08lx %08lx (%s)\n", |
370 | break; // security check | ||
371 | i += scnprintf(videocodec_buf + i, size -i -1, | ||
372 | "M %32s %04x %08lx %08lx (%s)\n", | ||
373 | a->codec->master_data->name, | 340 | a->codec->master_data->name, |
374 | a->codec->master_data->type, | 341 | a->codec->master_data->type, |
375 | a->codec->master_data->flags, | 342 | a->codec->master_data->flags, |
@@ -380,54 +347,21 @@ videocodec_build_table (void) | |||
380 | h = h->next; | 347 | h = h->next; |
381 | } | 348 | } |
382 | 349 | ||
383 | return i; | 350 | return 0; |
384 | } | 351 | } |
385 | 352 | ||
386 | //The definition: | 353 | static int proc_videocodecs_open(struct inode *inode, struct file *file) |
387 | //typedef int (read_proc_t)(char *page, char **start, off_t off, | ||
388 | // int count, int *eof, void *data); | ||
389 | |||
390 | static int | ||
391 | videocodec_info (char *buffer, | ||
392 | char **buffer_location, | ||
393 | off_t offset, | ||
394 | int buffer_length, | ||
395 | int *eof, | ||
396 | void *data) | ||
397 | { | 354 | { |
398 | int size; | 355 | return single_open(file, proc_videocodecs_show, NULL); |
399 | |||
400 | dprintk(3, "videocodec_info: offset: %ld, len %d / size %d\n", | ||
401 | offset, buffer_length, videocodec_bufsize); | ||
402 | |||
403 | if (offset == 0) { | ||
404 | videocodec_bufsize = videocodec_build_table(); | ||
405 | } | ||
406 | if ((offset < 0) || (offset >= videocodec_bufsize)) { | ||
407 | dprintk(4, | ||
408 | "videocodec_info: call delivers no result, return 0\n"); | ||
409 | *eof = 1; | ||
410 | return 0; | ||
411 | } | ||
412 | |||
413 | if (buffer_length < (videocodec_bufsize - offset)) { | ||
414 | dprintk(4, "videocodec_info: %ld needed, %d got\n", | ||
415 | videocodec_bufsize - offset, buffer_length); | ||
416 | size = buffer_length; | ||
417 | } else { | ||
418 | dprintk(4, "videocodec_info: last reading of %ld bytes\n", | ||
419 | videocodec_bufsize - offset); | ||
420 | size = videocodec_bufsize - offset; | ||
421 | *eof = 1; | ||
422 | } | ||
423 | |||
424 | memcpy(buffer, videocodec_buf + offset, size); | ||
425 | /* doesn't work... */ | ||
426 | /* copy_to_user(buffer, videocodec_buf+offset, size); */ | ||
427 | /* *buffer_location = videocodec_buf+offset; */ | ||
428 | |||
429 | return size; | ||
430 | } | 356 | } |
357 | |||
358 | static const struct file_operations videocodecs_proc_fops = { | ||
359 | .owner = THIS_MODULE, | ||
360 | .open = proc_videocodecs_open, | ||
361 | .read = seq_read, | ||
362 | .llseek = seq_lseek, | ||
363 | .release = single_release, | ||
364 | }; | ||
431 | #endif | 365 | #endif |
432 | 366 | ||
433 | /* ===================== */ | 367 | /* ===================== */ |
@@ -444,16 +378,8 @@ videocodec_init (void) | |||
444 | VIDEOCODEC_VERSION); | 378 | VIDEOCODEC_VERSION); |
445 | 379 | ||
446 | #ifdef CONFIG_PROC_FS | 380 | #ifdef CONFIG_PROC_FS |
447 | videocodec_buf = NULL; | 381 | videocodec_proc_entry = proc_create("videocodecs", 0, NULL, &videocodecs_proc_fops); |
448 | videocodec_bufsize = 0; | 382 | if (!videocodec_proc_entry) { |
449 | |||
450 | videocodec_proc_entry = create_proc_entry("videocodecs", 0, NULL); | ||
451 | if (videocodec_proc_entry) { | ||
452 | videocodec_proc_entry->read_proc = videocodec_info; | ||
453 | videocodec_proc_entry->write_proc = NULL; | ||
454 | videocodec_proc_entry->data = NULL; | ||
455 | videocodec_proc_entry->owner = THIS_MODULE; | ||
456 | } else { | ||
457 | dprintk(1, KERN_ERR "videocodec: can't init procfs.\n"); | 383 | dprintk(1, KERN_ERR "videocodec: can't init procfs.\n"); |
458 | } | 384 | } |
459 | #endif | 385 | #endif |
@@ -465,7 +391,6 @@ videocodec_exit (void) | |||
465 | { | 391 | { |
466 | #ifdef CONFIG_PROC_FS | 392 | #ifdef CONFIG_PROC_FS |
467 | remove_proc_entry("videocodecs", NULL); | 393 | remove_proc_entry("videocodecs", NULL); |
468 | kfree(videocodec_buf); | ||
469 | #endif | 394 | #endif |
470 | } | 395 | } |
471 | 396 | ||