Fix potential memory leak in tipc_named_node_up()

There seems to be a memory leak in net/tipc/name_distr.c::tipc_named_node_up()

The function, with comments, is this :

void tipc_named_node_up(unsigned long node)

{
        struct publication *publ;
        struct distr_item *item = NULL;
        struct sk_buff *buf = NULL;
        u32 left = 0;
        u32 rest;
        u32 max_item_buf;

        read_lock_bh(&tipc_nametbl_lock);
        max_item_buf = TIPC_MAX_USER_MSG_SIZE / ITEM_SIZE;
        max_item_buf *= ITEM_SIZE;
        rest = publ_cnt * ITEM_SIZE;

        list_for_each_entry(publ, &publ_root, local_list) {
-----> If we stop processing here after doing 1, 2 & 3 below we end up at (4) (below).
                if (!buf) {
                        left = (rest <= max_item_buf) ? rest : max_item_buf;
                        rest -= left;
                        buf = named_prepare_buf(PUBLICATION, left, node);

-----> (1) here we allocate memory and store a pointer to it in 'buf'.

                        if (!buf) {
-----> (2) This test needs to fail, meaning we did allocate some memory.
                                warn("Bulk publication distribution failure\n");
                                goto exit;
                        }
                        item = (struct distr_item *)msg_data(buf_msg(buf));
                }
                publ_to_item(item, publ);
                item++;
                left -= ITEM_SIZE;
                if (!left) {
-----> (3) If this test fails we loop and do nothing to 'buf'.
                        msg_set_link_selector(buf_msg(buf), node);
                        dbg("tipc_named_node_up: sending publish msg to "
                            "<%u.%u.%u>\n", tipc_zone(node),
                            tipc_cluster(node), tipc_node(node));
                        tipc_link_send(buf, node, node);
                        buf = NULL;
                }
        }
exit:
        read_unlock_bh(&tipc_nametbl_lock);

-----> (4) here we return without freeing 'buf' - memory leak. }

Luckily this is easy to fix, since we can only leave the function with 'buf' either set to NULL or (in the leak case) set to a valid address, and since kfree() handles being passed NULL gracefully we can simply kfree(buf) just before we leave the function.

Quoted 48 lines · Jesper Juhl

There seems to be a memory leak in net/tipc/name_distr.c::tipc_named_node_up()

The function, with comments, is this :

void tipc_named_node_up(unsigned long node)

{
        struct publication *publ;
        struct distr_item *item = NULL;
        struct sk_buff *buf = NULL;
        u32 left = 0;
        u32 rest;
        u32 max_item_buf;

        read_lock_bh(&tipc_nametbl_lock);
        max_item_buf = TIPC_MAX_USER_MSG_SIZE / ITEM_SIZE;
        max_item_buf *= ITEM_SIZE;
        rest = publ_cnt * ITEM_SIZE;

        list_for_each_entry(publ, &publ_root, local_list) {
-----> If we stop processing here after doing 1, 2 & 3 below we end up at (4) (below).
                if (!buf) {
                        left = (rest <= max_item_buf) ? rest : max_item_buf;
                        rest -= left;
                        buf = named_prepare_buf(PUBLICATION, left, node);

-----> (1) here we allocate memory and store a pointer to it in 'buf'.

                        if (!buf) {
-----> (2) This test needs to fail, meaning we did allocate some memory.
                                warn("Bulk publication distribution failure\n");
                                goto exit;
                        }
                        item = (struct distr_item *)msg_data(buf_msg(buf));
                }
                publ_to_item(item, publ);
                item++;
                left -= ITEM_SIZE;
                if (!left) {
-----> (3) If this test fails we loop and do nothing to 'buf'.
                        msg_set_link_selector(buf_msg(buf), node);
                        dbg("tipc_named_node_up: sending publish msg to "
                            "<%u.%u.%u>\n", tipc_zone(node),
                            tipc_cluster(node), tipc_node(node));
                        tipc_link_send(buf, node, node);
                        buf = NULL;
                }
        }
exit:
        read_unlock_bh(&tipc_nametbl_lock);

Quoted 7 lines

-----> (4) here we return without freeing 'buf' - memory leak. }

Luckily this is easy to fix, since we can only leave the function with 'buf' either set to NULL or (in the leak case) set to a valid address, and since kfree() handles being passed NULL gracefully we can simply kfree(buf) just before we leave the function.

Actually, I don't think that there's a leak.

publ_cnt: Number of items in the list

rest = publ_cnt * ITEM_SIZE
max_item_buf = n * ITEM_SIZE (Buffer can hold n elements at most)

1)
If publ_cnt <= n, "rest" becomes 0 and "left" becomes publ_cnt * ITEM_SIZE, so for the last iteration "left" becomes 0 and "buf" is freed.

2)
And if publ_cnt > n, "left" becomes 0 in the nth iteration. As "rest" already got decrement by n * ITEM_SIZE, you now got:
rest = (publ_cnt - n) * ITEM_SIZE
Then after 2*n iterations:
rest = (publ_cnt - 2*n) * ITEM_SIZE
and so on, until publ_cnt - m*n < n

At that point, "left" becomes (publ_cnt - m*n) * ITEM_SIZE, and there are also (publ_cnt - m*n) iterations left, so "left" again becomes 0 in the last iteration and "buf" is freed.

Besides that, is it valid to call kfree() on a buffer allocated by alloc_skb()?