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

firmware loader: fix use-after-free by double abort

fw_priv->buf is accessed in both request_firmware_load() and
writing to sysfs file of 'loading' context, but not protected
by 'fw_lock' entirely. The patch makes sure that access on
'fw_priv->buf' is protected by the lock.

So fixes the double abort problem reported by nirinA raseliarison:

http://lkml.org/lkml/2013/6/14/188

Reported-and-tested-by: nirinA raseliarison <nirina.raseliarison@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: stable <stable@vger.kernel.org> # 3.9
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Ming Lei and committed by
Greg Kroah-Hartman
87597936 7d132055

+18 -9
+18 -9
drivers/base/firmware_class.c
··· 450 { 451 struct firmware_buf *buf = fw_priv->buf; 452 453 set_bit(FW_STATUS_ABORT, &buf->status); 454 complete_all(&buf->completion); 455 } 456 457 #define is_fw_load_aborted(buf) \ ··· 538 struct device_attribute *attr, char *buf) 539 { 540 struct firmware_priv *fw_priv = to_firmware_priv(dev); 541 - int loading = test_bit(FW_STATUS_LOADING, &fw_priv->buf->status); 542 543 return sprintf(buf, "%d\n", loading); 544 } ··· 585 const char *buf, size_t count) 586 { 587 struct firmware_priv *fw_priv = to_firmware_priv(dev); 588 - struct firmware_buf *fw_buf = fw_priv->buf; 589 int loading = simple_strtol(buf, NULL, 10); 590 int i; 591 592 mutex_lock(&fw_lock); 593 - 594 if (!fw_buf) 595 goto out; 596 ··· 792 struct firmware_priv, timeout_work.work); 793 794 mutex_lock(&fw_lock); 795 - if (test_bit(FW_STATUS_DONE, &(fw_priv->buf->status))) { 796 - mutex_unlock(&fw_lock); 797 - return; 798 - } 799 fw_load_abort(fw_priv); 800 mutex_unlock(&fw_lock); 801 } ··· 871 wait_for_completion(&buf->completion); 872 873 cancel_delayed_work_sync(&fw_priv->timeout_work); 874 - 875 - fw_priv->buf = NULL; 876 877 device_remove_file(f_dev, &dev_attr_loading); 878 err_del_bin_attr:
··· 450 { 451 struct firmware_buf *buf = fw_priv->buf; 452 453 + /* 454 + * There is a small window in which user can write to 'loading' 455 + * between loading done and disappearance of 'loading' 456 + */ 457 + if (test_bit(FW_STATUS_DONE, &buf->status)) 458 + return; 459 + 460 set_bit(FW_STATUS_ABORT, &buf->status); 461 complete_all(&buf->completion); 462 + 463 + /* avoid user action after loading abort */ 464 + fw_priv->buf = NULL; 465 } 466 467 #define is_fw_load_aborted(buf) \ ··· 528 struct device_attribute *attr, char *buf) 529 { 530 struct firmware_priv *fw_priv = to_firmware_priv(dev); 531 + int loading = 0; 532 + 533 + mutex_lock(&fw_lock); 534 + if (fw_priv->buf) 535 + loading = test_bit(FW_STATUS_LOADING, &fw_priv->buf->status); 536 + mutex_unlock(&fw_lock); 537 538 return sprintf(buf, "%d\n", loading); 539 } ··· 570 const char *buf, size_t count) 571 { 572 struct firmware_priv *fw_priv = to_firmware_priv(dev); 573 + struct firmware_buf *fw_buf; 574 int loading = simple_strtol(buf, NULL, 10); 575 int i; 576 577 mutex_lock(&fw_lock); 578 + fw_buf = fw_priv->buf; 579 if (!fw_buf) 580 goto out; 581 ··· 777 struct firmware_priv, timeout_work.work); 778 779 mutex_lock(&fw_lock); 780 fw_load_abort(fw_priv); 781 mutex_unlock(&fw_lock); 782 } ··· 860 wait_for_completion(&buf->completion); 861 862 cancel_delayed_work_sync(&fw_priv->timeout_work); 863 864 device_remove_file(f_dev, &dev_attr_loading); 865 err_del_bin_attr: