01 Aug, 2019

1 commit

  • …rigger_change_speed()

    In phy_led_trigger_change_speed(), there is an if statement on line 48
    to check whether phy->last_triggered is NULL:
    if (!phy->last_triggered)

    When phy->last_triggered is NULL, it is used on line 52:
    led_trigger_event(&phy->last_triggered->trigger, LED_OFF);

    Thus, a possible null-pointer dereference may occur.

    To fix this bug, led_trigger_event(&phy->last_triggered->trigger,
    LED_OFF) is called when phy->last_triggered is not NULL.

    This bug is found by a static analysis tool STCheck written by
    the OSLAB group in Tsinghua University.

    Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

    Jia-Ju Bai
     

23 Jan, 2019

1 commit


10 Nov, 2018

1 commit

  • The phy core provides a handy phy_speed_to_str() helper, so use that
    instead of doing our own formatting of the different known link speeds.
    To do this, increase PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE to 11 so we can fit
    'Unsupported' if necessary.

    Signed-off-by: Kyle Roeschley
    Signed-off-by: David S. Miller

    Kyle Roeschley
     

13 Jun, 2018

1 commit

  • The devm_kzalloc() function has a 2-factor argument form, devm_kcalloc().
    This patch replaces cases of:

    devm_kzalloc(handle, a * b, gfp)

    with:
    devm_kcalloc(handle, a * b, gfp)

    as well as handling cases of:

    devm_kzalloc(handle, a * b * c, gfp)

    with:

    devm_kzalloc(handle, array3_size(a, b, c), gfp)

    as it's slightly less ugly than:

    devm_kcalloc(handle, array_size(a, b), c, gfp)

    This does, however, attempt to ignore constant size factors like:

    devm_kzalloc(handle, 4 * 1024, gfp)

    though any constants defined via macros get caught up in the conversion.

    Any factors with a sizeof() of "unsigned char", "char", and "u8" were
    dropped, since they're redundant.

    Some manual whitespace fixes were needed in this patch, as Coccinelle
    really liked to write "=devm_kcalloc..." instead of "= devm_kcalloc...".

    The Coccinelle script used for this was:

    // Fix redundant parens around sizeof().
    @@
    expression HANDLE;
    type TYPE;
    expression THING, E;
    @@

    (
    devm_kzalloc(HANDLE,
    - (sizeof(TYPE)) * E
    + sizeof(TYPE) * E
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (sizeof(THING)) * E
    + sizeof(THING) * E
    , ...)
    )

    // Drop single-byte sizes and redundant parens.
    @@
    expression HANDLE;
    expression COUNT;
    typedef u8;
    typedef __u8;
    @@

    (
    devm_kzalloc(HANDLE,
    - sizeof(u8) * (COUNT)
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(__u8) * (COUNT)
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(char) * (COUNT)
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(unsigned char) * (COUNT)
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(u8) * COUNT
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(__u8) * COUNT
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(char) * COUNT
    + COUNT
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(unsigned char) * COUNT
    + COUNT
    , ...)
    )

    // 2-factor product with sizeof(type/expression) and identifier or constant.
    @@
    expression HANDLE;
    type TYPE;
    expression THING;
    identifier COUNT_ID;
    constant COUNT_CONST;
    @@

    (
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * (COUNT_ID)
    + COUNT_ID, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * COUNT_ID
    + COUNT_ID, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * (COUNT_CONST)
    + COUNT_CONST, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * COUNT_CONST
    + COUNT_CONST, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * (COUNT_ID)
    + COUNT_ID, sizeof(THING)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * COUNT_ID
    + COUNT_ID, sizeof(THING)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * (COUNT_CONST)
    + COUNT_CONST, sizeof(THING)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * COUNT_CONST
    + COUNT_CONST, sizeof(THING)
    , ...)
    )

    // 2-factor product, only identifiers.
    @@
    expression HANDLE;
    identifier SIZE, COUNT;
    @@

    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - SIZE * COUNT
    + COUNT, SIZE
    , ...)

    // 3-factor product with 1 sizeof(type) or sizeof(expression), with
    // redundant parens removed.
    @@
    expression HANDLE;
    expression THING;
    identifier STRIDE, COUNT;
    type TYPE;
    @@

    (
    devm_kzalloc(HANDLE,
    - sizeof(TYPE) * (COUNT) * (STRIDE)
    + array3_size(COUNT, STRIDE, sizeof(TYPE))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE) * (COUNT) * STRIDE
    + array3_size(COUNT, STRIDE, sizeof(TYPE))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE) * COUNT * (STRIDE)
    + array3_size(COUNT, STRIDE, sizeof(TYPE))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE) * COUNT * STRIDE
    + array3_size(COUNT, STRIDE, sizeof(TYPE))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING) * (COUNT) * (STRIDE)
    + array3_size(COUNT, STRIDE, sizeof(THING))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING) * (COUNT) * STRIDE
    + array3_size(COUNT, STRIDE, sizeof(THING))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING) * COUNT * (STRIDE)
    + array3_size(COUNT, STRIDE, sizeof(THING))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING) * COUNT * STRIDE
    + array3_size(COUNT, STRIDE, sizeof(THING))
    , ...)
    )

    // 3-factor product with 2 sizeof(variable), with redundant parens removed.
    @@
    expression HANDLE;
    expression THING1, THING2;
    identifier COUNT;
    type TYPE1, TYPE2;
    @@

    (
    devm_kzalloc(HANDLE,
    - sizeof(TYPE1) * sizeof(TYPE2) * COUNT
    + array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE1) * sizeof(THING2) * (COUNT)
    + array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING1) * sizeof(THING2) * COUNT
    + array3_size(COUNT, sizeof(THING1), sizeof(THING2))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(THING1) * sizeof(THING2) * (COUNT)
    + array3_size(COUNT, sizeof(THING1), sizeof(THING2))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE1) * sizeof(THING2) * COUNT
    + array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
    , ...)
    |
    devm_kzalloc(HANDLE,
    - sizeof(TYPE1) * sizeof(THING2) * (COUNT)
    + array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
    , ...)
    )

    // 3-factor product, only identifiers, with redundant parens removed.
    @@
    expression HANDLE;
    identifier STRIDE, SIZE, COUNT;
    @@

    (
    devm_kzalloc(HANDLE,
    - (COUNT) * STRIDE * SIZE
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - COUNT * (STRIDE) * SIZE
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - COUNT * STRIDE * (SIZE)
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (COUNT) * (STRIDE) * SIZE
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - COUNT * (STRIDE) * (SIZE)
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (COUNT) * STRIDE * (SIZE)
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (COUNT) * (STRIDE) * (SIZE)
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - COUNT * STRIDE * SIZE
    + array3_size(COUNT, STRIDE, SIZE)
    , ...)
    )

    // Any remaining multi-factor products, first at least 3-factor products,
    // when they're not all constants...
    @@
    expression HANDLE;
    expression E1, E2, E3;
    constant C1, C2, C3;
    @@

    (
    devm_kzalloc(HANDLE, C1 * C2 * C3, ...)
    |
    devm_kzalloc(HANDLE,
    - (E1) * E2 * E3
    + array3_size(E1, E2, E3)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (E1) * (E2) * E3
    + array3_size(E1, E2, E3)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - (E1) * (E2) * (E3)
    + array3_size(E1, E2, E3)
    , ...)
    |
    devm_kzalloc(HANDLE,
    - E1 * E2 * E3
    + array3_size(E1, E2, E3)
    , ...)
    )

    // And then all remaining 2 factors products when they're not all constants,
    // keeping sizeof() as the second factor argument.
    @@
    expression HANDLE;
    expression THING, E1, E2;
    type TYPE;
    constant C1, C2, C3;
    @@

    (
    devm_kzalloc(HANDLE, sizeof(THING) * C2, ...)
    |
    devm_kzalloc(HANDLE, sizeof(TYPE) * C2, ...)
    |
    devm_kzalloc(HANDLE, C1 * C2 * C3, ...)
    |
    devm_kzalloc(HANDLE, C1 * C2, ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * (E2)
    + E2, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(TYPE) * E2
    + E2, sizeof(TYPE)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * (E2)
    + E2, sizeof(THING)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - sizeof(THING) * E2
    + E2, sizeof(THING)
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - (E1) * E2
    + E1, E2
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - (E1) * (E2)
    + E1, E2
    , ...)
    |
    - devm_kzalloc
    + devm_kcalloc
    (HANDLE,
    - E1 * E2
    + E1, E2
    , ...)
    )

    Signed-off-by: Kees Cook

    Kees Cook
     

08 Nov, 2017

2 commits

  • Currently, we create a LED trigger for any link speed known to a PHY.
    These triggers only fire when their exact link speed had been negotiated
    (they aren't cumulative, that is, they don't fire for "their or any higher"
    link speed).

    What we are missing, however, is a trigger which will fire on any link
    speed known to the PHY. Such trigger can then be used for implementing a
    poor man's substitute of the "link" LED on boards that lack it.
    Let's add it.

    Signed-off-by: Maciej S. Szmigiero
    Signed-off-by: David S. Miller

    Maciej S. Szmigiero
     
  • Currently, phy_led_trigger_change_speed() is handling a "no link" condition
    like it was some kind of an error (using "goto" to a code at the function
    end).

    However, having no link at PHY is an ordinary operational state, so let's
    handle it in an appropriately named separate function so it is more obvious
    what the code is doing.

    Signed-off-by: Maciej S. Szmigiero
    Signed-off-by: David S. Miller

    Maciej S. Szmigiero
     

26 Jan, 2017

2 commits

  • includes , which is not really
    needed. Drop the include from , and add it to all users
    that didn't include it explicitly.

    Suggested-by: Andrew Lunn
    Signed-off-by: Geert Uytterhoeven
    Reviewed-by: Andrew Lunn
    Signed-off-by: David S. Miller

    Geert Uytterhoeven
     
  • phy_attach_direct() ignores errors returned by
    phy_led_triggers_register(). I think that's OK, as LED triggers can be
    considered a non-critical feature.

    However, this causes problems later:
    - phy_led_trigger_change_speed() will access the array
    phy_device.phy_led_triggers, which has been freed in the error path
    of phy_led_triggers_register(), which may lead to a crash.

    - phy_led_triggers_unregister() will access the same array, leading to
    crashes during s2ram or poweroff, like:

    Unable to handle kernel NULL pointer dereference at virtual address
    00000000
    ...
    [] (__list_del_entry_valid) from [] (led_trigger_unregister+0x34/0xcc)
    [] (led_trigger_unregister) from [] (phy_led_triggers_unregister+0x28/0x34)
    [] (phy_led_triggers_unregister) from [] (phy_detach+0x30/0x74)
    [] (phy_detach) from [] (sh_eth_close+0x64/0x9c)
    [] (sh_eth_close) from [] (dpm_run_callback+0x48/0xc8)

    or:

    list_del corruption. prev->next should be dede6540, but was 2e323931
    ------------[ cut here ]------------
    kernel BUG at lib/list_debug.c:52!
    ...
    [] (__list_del_entry_valid) from [] (led_trigger_unregister+0x34/0xcc)
    [] (led_trigger_unregister) from [] (phy_led_triggers_unregister+0x28/0x34)
    [] (phy_led_triggers_unregister) from [] (phy_detach+0x30/0x74)
    [] (phy_detach) from [] (sh_eth_close+0x6c/0xa4)
    [] (sh_eth_close) from [] (__dev_close_many+0xac/0xd0)

    To fix this, clear phy_device.phy_num_led_triggers in the error path of
    phy_led_triggers_register() fails.

    Note that the "No phy led trigger registered for speed" message will
    still be printed on link speed changes, which is a good cue that
    something went wrong with the LED triggers.

    Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy link state change")
    Signed-off-by: Geert Uytterhoeven
    Reviewed-by: Florian Fainelli
    Signed-off-by: David S. Miller

    Geert Uytterhoeven
     

26 Nov, 2016

1 commit

  • When phy_init_hw() fails at phy_attach_direct();
    - phy_detach() calls phy_led_triggers_unregister() without
    previous call of phy_led_triggers_register().
    - still call phy_led_triggers_register() and cause memory leak.

    Fixes: 2e0bc452f472 ("net: phy: leds: add support for led triggers on phy link state change")
    Signed-off-by: Woojung Huh
    Signed-off-by: David S. Miller

    Woojung Huh
     

18 Oct, 2016

1 commit

  • Create an option CONFIG_LED_TRIGGER_PHY (default n), which will create a
    set of led triggers for each instantiated PHY device. There is one LED
    trigger per link-speed, per-phy.
    The triggers are registered during phy_attach and unregistered during
    phy_detach.

    This allows for a user to configure their system to allow a set of LEDs
    not controlled by the phy to represent link state changes on the phy.
    LEDS controlled by the phy are unaffected.

    For example, we have a board where some of the leds in the
    RJ45 socket are controlled by the phy, but others are not. Using the
    triggers provided by this patch the leds not controlled by the phy can
    be configured to show the current speed of the ethernet connection. The
    leds controlled by the phy are unaffected.

    Signed-off-by: Josh Cartwright
    Signed-off-by: Nathan Sullivan
    Signed-off-by: Zach Brown
    Signed-off-by: David S. Miller

    Zach Brown