Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

vt: push the tty_lock down into the map handling

When we do this it becomes clear the lock we should be holding is the vc
lock, and in fact many of our other helpers are properly invoked this way.

We don't at this point guarantee not to race the keyboard code but the results
of that appear harmless and that was true before we started as well.

We now have no users of tty_lock in the console driver...

Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Alan Cox and committed by
Greg Kroah-Hartman
5d1a33fa 10af77c1

+96 -53
+91 -32
drivers/tty/vt/consolemap.c
··· 19 19 #include <linux/init.h> 20 20 #include <linux/tty.h> 21 21 #include <asm/uaccess.h> 22 + #include <linux/console.h> 22 23 #include <linux/consolemap.h> 23 24 #include <linux/vt_kern.h> 24 25 ··· 313 312 if (!access_ok(VERIFY_READ, arg, E_TABSZ)) 314 313 return -EFAULT; 315 314 315 + console_lock(); 316 316 for (i=0; i<E_TABSZ ; i++) { 317 317 unsigned char uc; 318 318 __get_user(uc, arg+i); ··· 321 319 } 322 320 323 321 update_user_maps(); 322 + console_unlock(); 324 323 return 0; 325 324 } 326 325 ··· 333 330 if (!access_ok(VERIFY_WRITE, arg, E_TABSZ)) 334 331 return -EFAULT; 335 332 333 + console_lock(); 336 334 for (i=0; i<E_TABSZ ; i++) 337 - { 338 - ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]); 339 - __put_user((ch & ~0xff) ? 0 : ch, arg+i); 340 - } 335 + { 336 + ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]); 337 + __put_user((ch & ~0xff) ? 0 : ch, arg+i); 338 + } 339 + console_unlock(); 341 340 return 0; 342 341 } 343 342 ··· 351 346 if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short))) 352 347 return -EFAULT; 353 348 349 + console_lock(); 354 350 for (i=0; i<E_TABSZ ; i++) { 355 351 unsigned short us; 356 352 __get_user(us, arg+i); ··· 359 353 } 360 354 361 355 update_user_maps(); 356 + console_unlock(); 362 357 return 0; 363 358 } 364 359 ··· 371 364 if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short))) 372 365 return -EFAULT; 373 366 367 + console_lock(); 374 368 for (i=0; i<E_TABSZ ; i++) 375 369 __put_user(p[i], arg+i); 370 + console_unlock(); 376 371 377 372 return 0; 378 373 } ··· 416 407 } 417 408 } 418 409 410 + /* Caller must hold the console lock */ 419 411 void con_free_unimap(struct vc_data *vc) 420 412 { 421 413 struct uni_pagedir *p; ··· 497 487 return 0; 498 488 } 499 489 500 - /* ui is a leftover from using a hashtable, but might be used again */ 501 - int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui) 490 + /* ui is a leftover from using a hashtable, but might be used again 491 + Caller must hold the lock */ 492 + static int con_do_clear_unimap(struct vc_data *vc, struct unimapinit *ui) 502 493 { 503 494 struct uni_pagedir *p, *q; 504 - 495 + 505 496 p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc; 506 - if (p && p->readonly) return -EIO; 497 + if (p && p->readonly) 498 + return -EIO; 499 + 507 500 if (!p || --p->refcount) { 508 501 q = kzalloc(sizeof(*p), GFP_KERNEL); 509 502 if (!q) { 510 - if (p) p->refcount++; 503 + if (p) 504 + p->refcount++; 511 505 return -ENOMEM; 512 506 } 513 507 q->refcount=1; ··· 525 511 return 0; 526 512 } 527 513 514 + int con_clear_unimap(struct vc_data *vc, struct unimapinit *ui) 515 + { 516 + int ret; 517 + console_lock(); 518 + ret = con_do_clear_unimap(vc, ui); 519 + console_unlock(); 520 + return ret; 521 + } 522 + 528 523 int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) 529 524 { 530 525 int err = 0, err1, i; 531 526 struct uni_pagedir *p, *q; 532 527 528 + console_lock(); 529 + 533 530 /* Save original vc_unipagdir_loc in case we allocate a new one */ 534 531 p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc; 535 - if (p->readonly) return -EIO; 532 + if (p->readonly) { 533 + console_unlock(); 534 + return -EIO; 535 + } 536 536 537 - if (!ct) return 0; 537 + if (!ct) { 538 + console_unlock(); 539 + return 0; 540 + } 538 541 539 542 if (p->refcount > 1) { 540 543 int j, k; 541 544 u16 **p1, *p2, l; 542 545 543 - err1 = con_clear_unimap(vc, NULL); 544 - if (err1) return err1; 546 + err1 = con_do_clear_unimap(vc, NULL); 547 + if (err1) { 548 + console_unlock(); 549 + return err1; 550 + } 545 551 546 552 /* 547 553 * Since refcount was > 1, con_clear_unimap() allocated a ··· 592 558 *vc->vc_uni_pagedir_loc = (unsigned long)p; 593 559 con_release_unimap(q); 594 560 kfree(q); 595 - return err1; 561 + console_unlock(); 562 + return err1; 596 563 } 597 564 } 598 565 } else { ··· 627 592 /* 628 593 * Merge with fontmaps of any other virtual consoles. 629 594 */ 630 - if (con_unify_unimap(vc, p)) 595 + if (con_unify_unimap(vc, p)) { 596 + console_unlock(); 631 597 return err; 598 + } 632 599 633 600 for (i = 0; i <= 3; i++) 634 601 set_inverse_transl(vc, p, i); /* Update inverse translations */ 635 602 set_inverse_trans_unicode(vc, p); 636 - 603 + 604 + console_unlock(); 637 605 return err; 638 606 } 639 607 640 - /* Loads the unimap for the hardware font, as defined in uni_hash.tbl. 641 - The representation used was the most compact I could come up 642 - with. This routine is executed at sys_setup time, and when the 643 - PIO_FONTRESET ioctl is called. */ 644 - 608 + /** 609 + * con_set_default_unimap - set default unicode map 610 + * @vc: the console we are updating 611 + * 612 + * Loads the unimap for the hardware font, as defined in uni_hash.tbl. 613 + * The representation used was the most compact I could come up 614 + * with. This routine is executed at video setup, and when the 615 + * PIO_FONTRESET ioctl is called. 616 + * 617 + * The caller must hold the console lock 618 + */ 645 619 int con_set_default_unimap(struct vc_data *vc) 646 620 { 647 621 int i, j, err = 0, err1; ··· 661 617 p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc; 662 618 if (p == dflt) 663 619 return 0; 620 + 664 621 dflt->refcount++; 665 622 *vc->vc_uni_pagedir_loc = (unsigned long)dflt; 666 623 if (p && !--p->refcount) { ··· 673 628 674 629 /* The default font is always 256 characters */ 675 630 676 - err = con_clear_unimap(vc, NULL); 677 - if (err) return err; 631 + err = con_do_clear_unimap(vc, NULL); 632 + if (err) 633 + return err; 678 634 679 635 p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc; 680 636 q = dfont_unitable; ··· 700 654 } 701 655 EXPORT_SYMBOL(con_set_default_unimap); 702 656 657 + /** 658 + * con_copy_unimap - copy unimap between two vts 659 + * @dst_vc: target 660 + * @src_vt: source 661 + * 662 + * The caller must hold the console lock when invoking this method 663 + */ 703 664 int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc) 704 665 { 705 666 struct uni_pagedir *q; ··· 721 668 *dst_vc->vc_uni_pagedir_loc = (long)q; 722 669 return 0; 723 670 } 671 + EXPORT_SYMBOL(con_copy_unimap); 724 672 673 + /** 674 + * con_get_unimap - get the unicode map 675 + * @vc: the console to read from 676 + * 677 + * Read the console unicode data for this console. Called from the ioctl 678 + * handlers. 679 + */ 725 680 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list) 726 681 { 727 682 int i, j, k, ect; 728 683 u16 **p1, *p2; 729 684 struct uni_pagedir *p; 685 + 686 + console_lock(); 730 687 731 688 ect = 0; 732 689 if (*vc->vc_uni_pagedir_loc) { ··· 757 694 } 758 695 } 759 696 __put_user(ect, uct); 697 + console_unlock(); 760 698 return ((ect <= ct) ? 0 : -ENOMEM); 761 - } 762 - 763 - void con_protect_unimap(struct vc_data *vc, int rdonly) 764 - { 765 - struct uni_pagedir *p = (struct uni_pagedir *)*vc->vc_uni_pagedir_loc; 766 - 767 - if (p) 768 - p->readonly = rdonly; 769 699 } 770 700 771 701 /* ··· 766 710 * which shouldn't be affected by G0/G1 switching, etc. 767 711 * If the user map still contains default values, i.e. the 768 712 * direct-to-font mapping, then assume user is using Latin1. 713 + * 714 + * FIXME: at some point we need to decide if we want to lock the table 715 + * update element itself via the keyboard_event_lock for consistency with the 716 + * keyboard driver as well as the consoles 769 717 */ 770 718 /* may be called during an interrupt */ 771 719 u32 conv_8bit_to_uni(unsigned char c) ··· 837 777 con_set_default_unimap(vc_cons[i].d); 838 778 } 839 779 840 - EXPORT_SYMBOL(con_copy_unimap);
+5 -20
drivers/tty/vt/vt_ioctl.c
··· 910 910 ret = con_font_op(vc_cons[fg_console].d, &op); 911 911 if (ret) 912 912 break; 913 + console_lock(); 913 914 con_set_default_unimap(vc_cons[fg_console].d); 915 + console_unlock(); 914 916 break; 915 917 } 916 918 #endif ··· 936 934 case PIO_SCRNMAP: 937 935 if (!perm) 938 936 ret = -EPERM; 939 - else { 940 - tty_lock(); 937 + else 941 938 ret = con_set_trans_old(up); 942 - tty_unlock(); 943 - } 944 939 break; 945 940 946 941 case GIO_SCRNMAP: 947 - tty_lock(); 948 942 ret = con_get_trans_old(up); 949 - tty_unlock(); 950 943 break; 951 944 952 945 case PIO_UNISCRNMAP: 953 946 if (!perm) 954 947 ret = -EPERM; 955 - else { 956 - tty_lock(); 948 + else 957 949 ret = con_set_trans_new(up); 958 - tty_unlock(); 959 - } 960 950 break; 961 951 962 952 case GIO_UNISCRNMAP: 963 - tty_lock(); 964 953 ret = con_get_trans_new(up); 965 - tty_unlock(); 966 954 break; 967 955 968 956 case PIO_UNIMAPCLR: ··· 962 970 ret = copy_from_user(&ui, up, sizeof(struct unimapinit)); 963 971 if (ret) 964 972 ret = -EFAULT; 965 - else { 966 - tty_lock(); 973 + else 967 974 con_clear_unimap(vc, &ui); 968 - tty_unlock(); 969 - } 970 975 break; 971 976 } 972 977 973 978 case PIO_UNIMAP: 974 979 case GIO_UNIMAP: 975 - tty_lock(); 976 980 ret = do_unimap_ioctl(cmd, up, perm, vc); 977 - tty_unlock(); 978 981 break; 979 982 980 983 case VT_LOCKSWITCH: ··· 1183 1196 1184 1197 case PIO_UNIMAP: 1185 1198 case GIO_UNIMAP: 1186 - tty_lock(); 1187 1199 ret = compat_unimap_ioctl(cmd, up, perm, vc); 1188 - tty_unlock(); 1189 1200 break; 1190 1201 1191 1202 /*
-1
include/linux/vt_kern.h
··· 70 70 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list); 71 71 int con_set_default_unimap(struct vc_data *vc); 72 72 void con_free_unimap(struct vc_data *vc); 73 - void con_protect_unimap(struct vc_data *vc, int rdonly); 74 73 int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc); 75 74 76 75 #define vc_translate(vc, c) ((vc)->vc_translate[(c) | \