Commit Graph

29 Commits

Author SHA1 Message Date
Maxim Petrov
575322b09c ip: fix memory leak in 'ip maddr show'
In `read_dev_mcast`, the list of ma_info is allocated, but not cleared
after use. Free the list in the end to make valgrind happy.

Detected by valgrind: "valgrind ./ip/ip maddr show"

Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com>
2023-10-16 09:15:04 -07:00
Stephen Hemminger
5c4697a405 whitespace cleanups
Remove trailing whitespace.
Make sure there is space after keywords.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2023-06-05 18:45:45 -07:00
Stephen Hemminger
8cda7a24a9 ipmaddr: fix dereference of NULL on malloc() failure
Found by -fanalyzer. This is a bug since beginning of initial
versions of ip multicast support (pre git).

ipmaddr.c: In function ‘read_dev_mcast’:
ipmaddr.c:105:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  105 |                         memcpy(ma, &m, sizeof(m));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |            ~ ~~~~~~~~~~~~~
           |      |            |       |
           |      |            |       (7) ...to here
           |      |            (8) following ‘true’ branch...
           |  276 |                 read_dev_mcast(&list);
           |      |                 ~~~~~~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_dev_mcast’ from ‘multiaddr_list’
           |
           +--> ‘read_dev_mcast’: events 11-12
                  |
                  |   82 | static void read_dev_mcast(struct ma_info **result_p)
                  |      |             ^~~~~~~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_dev_mcast’
                  |......
                  |   87 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |
                ‘read_dev_mcast’: event 13
                  |
                  |cc1:
                  | (13): ...to here
                  |
                ‘read_dev_mcast’: events 14-17
                  |
                  |   90 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (14) following ‘true’ branch...
                  |   91 |                 char hexa[256];
                  |   92 |                 struct ma_info m = { .addr.family = AF_PACKET };
                  |      |                                ~
                  |      |                                |
                  |      |                                (15) ...to here
                  |......
                  |  103 |                         struct ma_info *ma = malloc(sizeof(m));
                  |      |                                              ~~~~~~~~~~~~~~~~~
                  |      |                                              |
                  |      |                                              (16) this call could return NULL
                  |  104 |
                  |  105 |                         memcpy(ma, &m, sizeof(m));
                  |      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                         |
                  |      |                         (17) ‘ma’ could be NULL: unchecked value from (16)
                  |
ipmaddr.c: In function ‘read_igmp’:
ipmaddr.c:152:17: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  152 |                 memcpy(ma, &m, sizeof(m));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |              ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (7) ...to here
           |  276 |                 read_dev_mcast(&list);
           |  277 |         if (!filter.family || filter.family == AF_INET)
           |      |            ~
           |      |            |
           |      |            (8) following ‘true’ branch...
           |  278 |                 read_igmp(&list);
           |      |                 ~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_igmp’ from ‘multiaddr_list’
           |
           +--> ‘read_igmp’: events 11-14
                  |
                  |  116 | static void read_igmp(struct ma_info **result_p)
                  |      |             ^~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_igmp’
                  |......
                  |  126 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |  127 |                 return;
                  |  128 |         if (!fgets(buf, sizeof(buf), fp)) {
                  |      |            ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |            | |
                  |      |            | (13) ...to here
                  |      |            (14) following ‘false’ branch...
                  |
                ‘read_igmp’: event 15
                  |
                  |cc1:
                  | (15): ...to here
                  |
                ‘read_igmp’: events 16-19
                  |
                  |  133 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (16) following ‘true’ branch...
                  |......
                  |  136 |                 if (buf[0] != '\t') {
                  |      |                     ~~~~~~
                  |      |                        |
                  |      |                        (17) ...to here
                  |......
                  |  151 |                 ma = malloc(sizeof(m));
                  |      |                      ~~~~~~~~~~~~~~~~~
                  |      |                      |
                  |      |                      (18) this call could return NULL
                  |  152 |                 memcpy(ma, &m, sizeof(m));
                  |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                 |
                  |      |                 (19) ‘ma’ could be NULL: unchecked value from (18)
                  |
ipmaddr.c: In function ‘read_igmp6’:
ipmaddr.c:181:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  181 |                         memcpy(ma, &m, sizeof(m));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |              ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (7) ...to here
           |......
           |  279 |         if (!filter.family || filter.family == AF_INET6)
           |      |            ~
           |      |            |
           |      |            (8) following ‘true’ branch...
           |  280 |                 read_igmp6(&list);
           |      |                 ~~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_igmp6’ from ‘multiaddr_list’
           |
           +--> ‘read_igmp6’: events 11-12
                  |
                  |  159 | static void read_igmp6(struct ma_info **result_p)
                  |      |             ^~~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_igmp6’
                  |......
                  |  164 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |
                ‘read_igmp6’: event 13
                  |
                  |cc1:
                  | (13): ...to here
                  |
                ‘read_igmp6’: events 14-17
                  |
                  |  167 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (14) following ‘true’ branch...
                  |  168 |                 char hexa[256];
                  |  169 |                 struct ma_info m = { .addr.family = AF_INET6 };
                  |      |                                ~
                  |      |                                |
                  |      |                                (15) ...to here
                  |......
                  |  179 |                         struct ma_info *ma = malloc(sizeof(m));
                  |      |                                              ~~~~~~~~~~~~~~~~~
                  |      |                                              |
                  |      |                                              (16) this call could return NULL
                  |  180 |
                  |  181 |                         memcpy(ma, &m, sizeof(m));
                  |      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                         |
                  |      |                         (17) ‘ma’ could be NULL: unchecked value from (16)
                  |

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2023-05-13 19:02:41 -07:00
Stephen Hemminger
c37d21944b ip: use SPDX
Use SPDX instead of boilerplate text for ip and related
sub commands.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2023-01-14 09:00:34 -08:00
Sascha Hauer
7e7a1d107b iproute2: ip maddress: Check multiaddr length
ip maddress add|del takes a MAC address as argument, so insist on
getting a length of ETH_ALEN bytes. This makes sure the passed argument
is actually a MAC address and especially not an IPv4 address which
was previously accepted and silently taken as a MAC address.

While at it, do not print *argv in the error path as this has been
modified by ll_addr_a2n() and doesn't contain the full string anymore,
which can lead to misleading error messages.

Also while at it, replace the hardcoded buffer size with the actual
buffer size using sizeof().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2020-08-22 21:12:30 -07:00
Matteo Croce
8589eb4efd treewide: refactor help messages
Every tool in the iproute2 package have one or more function to show
an help message to the user. Some of these functions print the help
line by line with a series of printf call, e.g. ip/xfrm_state.c does
60 fprintf calls.
If we group all the calls to a single one and just concatenate strings,
we save a lot of libc calls and thus object size. The size difference
of the compiled binaries calculated with bloat-o-meter is:

        ip/ip:
        add/remove: 0/0 grow/shrink: 5/15 up/down: 103/-4796 (-4693)
        Total: Before=672591, After=667898, chg -0.70%
        ip/rtmon:
        add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-54 (-54)
        Total: Before=48879, After=48825, chg -0.11%
        tc/tc:
        add/remove: 0/2 grow/shrink: 31/10 up/down: 882/-6133 (-5251)
        Total: Before=351912, After=346661, chg -1.49%
        bridge/bridge:
        add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-459 (-459)
        Total: Before=70502, After=70043, chg -0.65%
        misc/lnstat:
        add/remove: 0/1 grow/shrink: 1/0 up/down: 48/-486 (-438)
        Total: Before=9960, After=9522, chg -4.40%
        tipc/tipc:
        add/remove: 0/0 grow/shrink: 1/1 up/down: 18/-62 (-44)
        Total: Before=79182, After=79138, chg -0.06%

While at it, indent some strings which were starting at column 0,
and use tabs where possible, to have a consistent style across helps.

Signed-off-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
2019-05-20 14:35:07 -07:00
Stephen Hemminger
b85076cd74 lib: introduce print_nl
Common pattern in iproute commands is to print a line seperator
in non-json mode. Make that a simple function.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2018-09-11 08:29:33 -07:00
Mahesh Bandewar
a5aaca9be2 ipmaddr: use preferred_family when given
When creating socket() AF_INET is used irrespective of the family
that is given at the command-line (with -4, -6, or -0). This change
will open the socket with the preferred family.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2018-08-30 07:57:11 -07:00
Stephen Hemminger
311dca0aa0 ipmaddr: json and color support
Support printing mulitcast addresses in json and color mode.
Output format is unchanged for normal use.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Ahern <dsahern@gmail.com>
2018-03-11 18:52:06 -07:00
Stephen Hemminger
913352fe54 drop unneeded include of syslog.h
Only arpd uses syslog

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
2017-11-12 16:22:36 -08:00
Michal Kubecek
21503ed2af ip maddr: fix filtering by device
Commit 530903dd90 ("ip: fix igmp parsing when iface is long") uses
variable len to keep trailing colon from interface name comparison.  This
variable is local to loop body but we set it in one pass and use it in
following one(s) so that we are actually using (pseudo)random length for
comparison. This became apparent since commit b48a1161f5 ("ipmaddr: Avoid
accessing uninitialized data") always initializes len to zero so that the
name comparison is always true. As a result, "ip maddr show dev eth0" shows
IPv4 multicast addresses for all interfaces.

Instead of keeping the length, let's simply replace the trailing colon with
a null byte. The bonus is that we get correct interface name in ma.name.

Fixes: 530903dd90 ("ip: fix igmp parsing when iface is long")
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Acked-by: Phil Sutter <phil@nwl.cc>
Acked-by: Petr Vorel <pvorel@suse.cz>
2017-10-21 15:02:24 +02:00
Phil Sutter
625df645b7 Check user supplied interface name lengths
The original problem was that something like:

| strncpy(ifr.ifr_name, *argv, IFNAMSIZ);

might leave ifr.ifr_name unterminated if length of *argv exceeds
IFNAMSIZ. In order to fix this, I thought about replacing all those
cases with (equivalent) calls to snprintf() or even introducing
strlcpy(). But as Ulrich Drepper correctly pointed out when rejecting
the latter from being added to glibc, truncating a string without
notifying the user is not to be considered good practice. So let's
excercise what he suggested and reject empty, overlong or otherwise
invalid interface names right from the start - this way calls to
strncpy() like shown above become safe and the user has a chance to
reconsider what he was trying to do.

Note that this doesn't add calls to check_ifname() to all places where
user supplied interface name is parsed. In many cases, the interface
must exist already and is therefore looked up using ll_name_to_index(),
so if_nametoindex() will perform the necessary checks already.

Signed-off-by: Phil Sutter <phil@nwl.cc>
2017-10-02 08:01:21 -07:00
Phil Sutter
b48a1161f5 ipmaddr: Avoid accessing uninitialized data
Looks like this can only happen if /proc/net/igmp is malformed, but
better be sure.

Signed-off-by: Phil Sutter <phil@nwl.cc>
2017-08-21 17:17:00 -07:00
Petr Vorel
530903dd90 ip: fix igmp parsing when iface is long
Entries with long vhost names in /proc/net/igmp have no whitespace
between name and colon, so sscanf() adds it to vhost and
'ip maddr show iface' doesn't include inet result.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
2017-01-17 08:39:55 -08:00
Phil Sutter
d17b136f7d Use C99 style initializers everywhere
This big patch was compiled by vimgrepping for memset calls and changing
to C99 initializer if applicable. One notable exception is the
initialization of union bpf_attr in tc/tc_bpf.c: changing it would break
for older gcc versions (at least <=3.4.6).

Calls to memset for struct rtattr pointer fields for parse_rtattr*()
were just dropped since they are not needed.

The changes here allowed the compiler to discover some unused variables,
so get rid of them, too.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: David Ahern <dsa@cumulusnetworks.com>
2016-07-20 12:05:24 -07:00
Phil Sutter
a418e45164 make format_host non-reentrant by default
There are only three users which require it to be reentrant, the rest is
fine without. Instead, provide a reentrant format_host_r() for users
which need it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
2016-03-27 10:37:34 -07:00
Stephen Hemminger
56f5daac98 ip: code cleanup
Run all the ip code through checkpatch and have it fix the obvious stuff.
2016-03-21 11:52:19 -07:00
Christoph Schulz
8aacb9bbbd ip: allow using a device "help" (or a prefix thereof)
Device names that match "help" or a prefix thereof should be allowed anywhere
a device name can be used. Note that a suitable keyword ("dev" or "name", the
latter for "ip tunnel") has to be used in these cases to resolve ambiguities.

Signed-off-by: Christoph Schulz <develop@kristov.de>
Reported-by: Leonhard Preis <leonhard@pre.is>
Reported-by: Wilhelm Wijkander <lists@0x5e.se>
2015-10-07 10:35:17 +01:00
Stephen Hemminger
d947b2384e ipmaddr: add whitespace around =
fix warning from parser
2013-03-14 13:44:25 -07:00
Stephen Hemminger
d1f28cf181 ip: make local functions static 2013-02-12 11:38:35 -08:00
Thomas Jarosch
19bcc05bea Fix file descriptor leak on error in read_igmp()
Detected by cppcheck.

Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
2011-10-07 11:20:23 -07:00
Brandon Philips
1f7190db39 ip: fix memory leak in ipmaddr.c
If the continue is taken, then there is a memory leak.

https://bugzilla.novell.com/show_bug.cgi?id=538996

Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Brandon Philips <bphilips@suse.de>
2011-04-12 14:23:52 -07:00
Stephen Hemminger
38c867d2a8 Add checks for fgets() when reading proc
If expected proc headers are missing, catch and print error.
2011-04-12 14:23:17 -07:00
Olivier Fourdan
7f71c0cae2 ip maddr show” on an infiniband address causes a stack corruption
“ip maddr show” on an infiniband address causes a stack corruption
because the length of the address for Infiniband (20 bytes, as
described in kernel doc Documentation/infiniband/ipoib.txt) does not
fit on the 16 bytes of the field in which it gets stored.

The proposed patch increases the size of the hardware address from 4
__u32 to 8 and also adds a check to avoid overriding the available
size while parsing the hardware address.

This bug affects current upstream code AFAICT.

Hope this helps,
Cheers,
Olivier.

“ip maddr show ib0” causes a stack corruption because the length of the address
for Infiniband (20 see kernel doc Documentation/infiniband/ipoib.txt) does not
fit on the 16 bytes of the field in which it gets stored.

The proposed patch increases the size of the hardware address from 4 u32 to 8
and adds a check to avoid overriding the available size while parsing the
hardware address.
2009-01-06 18:56:03 -08:00
Stephen Hemminger
ae665a522b Remove trailing whitespace
Go through source files and remove all trailing whitespace

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
2006-12-05 10:10:22 -08:00
shemminger
f332d16924 Cleanup GCC4 warnings about signedness. 2005-07-05 22:37:15 +00:00
osdl.org!shemminger
e5779fb2c8 more problems when kernel headers included before user headers.
(Logical change 1.29)
2004-06-09 22:56:28 +00:00
osdl.org!shemminger
aba5acdfdb (Logical change 1.3) 2004-04-15 20:56:59 +00:00
osdl.org!shemminger
86fdf0e47b Initial revision 2004-04-15 20:56:59 +00:00