• Yves-Alexis Perez's avatar
    firmware: fix usermode helper fallback loading · 2e700f8d
    Yves-Alexis Perez authored
    When you use the firmware usermode helper fallback with a timeout value set to a
    value greater than INT_MAX (2147483647) a cast overflow issue causes the
    timeout value to go negative and breaks all usermode helper loading. This
    regression was introduced through commit 68ff2a00 ("firmware_loader:
    handle timeout via wait_for_completion_interruptible_timeout()") on kernel
    v4.0.
    
    The firmware_class drivers relies on the firmware usermode helper
    fallback as a mechanism to look for firmware if the direct filesystem
    search failed only if:
    
      a) You've enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK (not many distros):
    
      Then all of these callers will rely on the fallback mechanism in case
      the firmware is not found through an initial direct filesystem lookup:
    
      o request_firmware()
      o request_firmware_into_buf()
      o request_firmware_nowait()
    
      b) If you've only enabled CONFIG_FW_LOADER_USER_HELPER (most distros):
    
      Then only callers using request_firmware_nowait() with the second
      argument set to false, this explicitly is requesting the UMH firmware
      fallback to be relied on in case the first filesystem lookup fails.
    
      Using Coccinelle SmPL grammar we have identified only two drivers
      explicitly requesting the UMH firmware fallback mechanism:
    
      - drivers/firmware/dell_rbu.c
      - drivers/leds/leds-lp55xx-common.c
    
    Since most distributions only enable CONFIG_FW_LOADER_USER_HELPER the
    biggest impact of this regression are users of the dell_rbu and
    leds-lp55xx-common device driver which required the UMH to find their
    respective needed firmwares.
    
    The default timeout for the UMH is set to 60 seconds always, as of
    commit 68ff2a00 ("firmware_loader: handle timeout via
    wait_for_completion_interruptible_timeout()") the timeout was bumped
    to MAX_JIFFY_OFFSET ((LONG_MAX >> 1)-1). Additionally the MAX_JIFFY_OFFSET
    value was also used if the timeout was configured by a user to 0.
    
    The following works:
    
    echo 2147483647 > /sys/class/firmware/timeout
    
    But both of the following set the timeout to MAX_JIFFY_OFFSET even if
    we display 0 back to userspace:
    
    echo 2147483648 > /sys/class/firmware/timeout
    cat /sys/class/firmware/timeout
    0
    
    echo 0> /sys/class/firmware/timeout
    cat /sys/class/firmware/timeout
    0
    
    A max value of INT_MAX (2147483647) seconds is therefore implicit due to the
    another cast with simple_strtol().
    
    This fixes the secondary cast (the first one is simple_strtol() but its an
    issue only by forcing an implicit limit) by re-using the timeout variable and
    only setting retval in appropriate cases.
    
    Lastly worth noting systemd had ripped out the UMH firmware fallback
    mechanism from udev since udev 2014 via commit be2ea723b1d023b3d
    ("udev: remove userspace firmware loading support"), so as of systemd v217.
    Signed-off-by: default avatarYves-Alexis Perez <corsac@corsac.net>
    Fixes: 68ff2a00 "firmware_loader: handle timeout via wait_for_completion_interruptible_timeout()"
    Cc: Luis R. Rodriguez <mcgrof@kernel.org>
    Cc: Ming Lei <ming.lei@canonical.com>
    Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Cc: stable@vger.kernel.org
    Acked-by: default avatarLuis R. Rodriguez <mcgrof@kernel.org>
    Reviewed-by: default avatarBjorn Andersson <bjorn.andersson@linaro.org>
    [mcgrof@kernel.org: gave commit log a whole lot of love]
    Signed-off-by: default avatarLuis R. Rodriguez <mcgrof@kernel.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    2e700f8d
firmware_class.c 41.8 KB