04 Jun, 2008

1 commit

  • From: Jarek Poplawski

    There is only one function in AX25 calling skb_append(), and it really
    looks suspicious: appends skb after previously enqueued one, but in
    the meantime this previous skb could be removed from the queue.

    This patch Fixes it the simple way, so this is not fully compatible with
    the current method, but testing hasn't shown any problems.

    Signed-off-by: Ralf Baechle
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

14 Apr, 2008

1 commit


13 Apr, 2008

1 commit

  • The ax25_uid_free call walks the ax25_uid_list and releases entries
    from it. The problem is that after the fisrt call to hlist_del_init
    the hlist_for_each_entry (which hides behind the ax25_uid_for_each)
    will consider the current position to be the last and will return.

    Thus, the whole list will be left not freed.

    Signed-off-by: Pavel Emelyanov
    Signed-off-by: David S. Miller

    Pavel Emelyanov
     

28 Mar, 2008

1 commit


26 Mar, 2008

3 commits


18 Feb, 2008

1 commit

  • According to some OOPS reports ax25_kick tries to clone NULL skbs
    sometimes. It looks like a race with ax25_clear_queues(). Probably
    there is no need to add more than a simple check for this yet.
    Another report suggested there are probably also cases where ax25
    ->paclen == 0 can happen in ax25_output(); this wasn't confirmed
    during testing but let's leave this debugging check for some time.

    Reported-and-tested-by: Jann Traschewski
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

13 Feb, 2008

4 commits

  • This patch changes current use of: init_timer(), add_timer()
    and del_timer() to setup_timer() with mod_timer(), which
    should be safer anyway.

    Reported-by: Jann Traschewski
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     
  • According to one of Jann's OOPS reports it looks like
    BUG_ON(timer_pending(timer)) triggers during add_timer()
    in ax25_start_t1timer(). This patch changes current use
    of: init_timer(), add_timer() and del_timer() to
    setup_timer() with mod_timer(), which should be safer
    anyway.

    Reported-by: Jann Traschewski
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     
  • > =================================
    > [ INFO: inconsistent lock state ]
    > 2.6.24-dg8ngn-p02 #1
    > ---------------------------------
    > inconsistent {softirq-on-W} -> {in-softirq-R} usage.
    > linuxnet/3046 [HC0[0]:SC1[2]:HE1:SE0] takes:
    > (ax25_route_lock){--.+}, at: [] ax25_get_route+0x18/0xb7 [ax25]
    > {softirq-on-W} state was registered at:
    ...

    This lockdep report shows that ax25_route_lock is taken for reading in
    softirq context, and for writing in process context with BHs enabled.
    So, to make this safe, all write_locks in ax25_route.c are changed to
    _bh versions.

    Reported-by: Jann Traschewski ,
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     
  • This lockdep warning:

    > =======================================================
    > [ INFO: possible circular locking dependency detected ]
    > 2.6.24 #3
    > -------------------------------------------------------
    > swapper/0 is trying to acquire lock:
    > (ax25_list_lock){-+..}, at: [] ax25_destroy_socket+0x171/0x1f0 [ax25]
    >
    > but task is already holding lock:
    > (slock-AF_AX25){-+..}, at: [] ax25_std_heartbeat_expiry+0x1c/0xe0 [ax25]
    >
    > which lock already depends on the new lock.
    ...

    shows that ax25_list_lock and slock-AF_AX25 are taken in different
    order: ax25_info_show() takes slock (bh_lock_sock(ax25->sk)) while
    ax25_list_lock is held, so reversely to other functions. To fix this
    the sock lock should be moved to ax25_info_start(), and there would
    be still problem with breaking ax25_list_lock (it seems this "proper"
    order isn't optimal yet). But, since it's only for reading proc info
    it seems this is not necessary (e.g. ax25_send_to_raw() does similar
    reading without this lock too).

    So, this patch removes sock lock to avoid deadlock possibility; there
    is also used sock_i_ino() function, which reads sk_socket under proper
    read lock. Additionally printf format of this i_ino is changed to %lu.

    Reported-by: Bernard Pidoux F6BVP
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

01 Feb, 2008

1 commit


29 Jan, 2008

3 commits

  • net/ax25/ax25_route.c:251:13: warning: context imbalance in
    'ax25_rt_seq_start' - wrong count at exit
    net/ax25/ax25_route.c:276:13: warning: context imbalance in 'ax25_rt_seq_stop'
    - unexpected unlock
    net/ax25/ax25_std_timer.c:65:25: warning: expensive signed divide
    net/ax25/ax25_uid.c:46:1: warning: symbol 'ax25_uid_list' was not declared.
    Should it be static?
    net/ax25/ax25_uid.c:146:13: warning: context imbalance in 'ax25_uid_seq_start'
    - wrong count at exit
    net/ax25/ax25_uid.c:169:13: warning: context imbalance in 'ax25_uid_seq_stop'
    - unexpected unlock
    net/ax25/af_ax25.c:573:28: warning: expensive signed divide
    net/ax25/af_ax25.c:1865:13: warning: context imbalance in 'ax25_info_start' -
    wrong count at exit
    net/ax25/af_ax25.c:1888:13: warning: context imbalance in 'ax25_info_stop' -
    unexpected unlock
    net/ax25/ax25_ds_timer.c:133:25: warning: expensive signed divide

    Signed-off-by: Eric Dumazet
    Signed-off-by: David S. Miller

    Eric Dumazet
     
  • This one is almost the same as the hunks in the
    first patch, but ax25 tables are created dynamically.

    So this patch differs a bit to handle this case.

    Signed-off-by: Pavel Emelyanov
    Signed-off-by: David S. Miller

    Pavel Emelyanov
     
  • Many-many code in the kernel initialized the timer->function
    and timer->data together with calling init_timer(timer). There
    is already a helper for this. Use it for networking code.

    The patch is HUGE, but makes the code 130 lines shorter
    (98 insertions(+), 228 deletions(-)).

    Signed-off-by: Pavel Emelyanov
    Acked-by: Arnaldo Carvalho de Melo
    Signed-off-by: David S. Miller

    Pavel Emelyanov
     

11 Jan, 2008

1 commit

  • Bernard Pidoux F6BVP reported:
    > When I killall kissattach I can see the following message.
    >
    > This happens on kernel 2.6.24-rc5 already patched with the 6 previously
    > patches I sent recently.
    >
    >
    > =======================================================
    > [ INFO: possible circular locking dependency detected ]
    > 2.6.23.9 #1
    > -------------------------------------------------------
    > kissattach/2906 is trying to acquire lock:
    > (linkfail_lock){-+..}, at: [] ax25_link_failed+0x11/0x39 [ax25]
    >
    > but task is already holding lock:
    > (ax25_list_lock){-+..}, at: [] ax25_device_event+0x38/0x84
    > [ax25]
    >
    > which lock already depends on the new lock.
    >
    >
    > the existing dependency chain (in reverse order) is:
    ...

    lockdep is worried about the different order here:

    #1 (rose_neigh_list_lock){-+..}:
    #3 (ax25_list_lock){-+..}:

    #0 (linkfail_lock){-+..}:
    #1 (rose_neigh_list_lock){-+..}:

    #3 (ax25_list_lock){-+..}:
    #0 (linkfail_lock){-+..}:

    So, ax25_list_lock could be taken before and after linkfail_lock.
    I don't know if this three-thread clutch is very probable (or
    possible at all), but it seems another bug reported by Bernard
    ("[...] system impossible to reboot with linux-2.6.24-rc5")
    could have similar source - namely ax25_list_lock held by
    ax25_kill_by_device() during ax25_disconnect(). It looks like the
    only place which calls ax25_disconnect() this way, so I guess, it
    isn't necessary.

    This patch is breaking the lock for ax25_disconnect().

    Reported-and-tested-by: Bernard Pidoux
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

10 Jan, 2008

1 commit

  • sfuzz can easily trigger any of those.

    move the printk message to the corresponding comment: makes the
    intention of the code clear and easy to pick up on an scheduled
    removal. as bonus simplify the braces placement.

    Signed-off-by: maximilian attems
    Signed-off-by: David S. Miller

    maximilian attems
     

20 Dec, 2007

1 commit


17 Dec, 2007

1 commit

  • Bernard Pidoux reported these lockdep warnings:

    [ INFO: possible irq lock inversion dependency detected ]
    2.6.23.1 #1
    ---------------------------------------------------------
    fpac/4933 just changed the state of lock:
    (slock-AF_AX25){--..}, at: [] ax25_disconnect+0x46/0xaf
    [ax25]
    but this lock was taken by another, soft-irq-safe lock in the past:
    (ax25_list_lock){-+..}

    and interrupts could create inverse lock ordering between them.
    [...]

    [ INFO: inconsistent lock state ]
    2.6.23.1 #1
    ---------------------------------
    inconsistent {in-softirq-W} -> {softirq-on-W} usage.
    ax25_call/4005 [HC0[0]:SC0[0]:HE1:SE1] takes:
    (slock-AF_AX25){-+..}, at: [] ax25_disconnect+0x46/0xaf [ax25]
    [...]

    This means slock-AF_AX25 could be taken both from softirq and process
    context with softirqs enabled, so it's endangered itself, but also makes
    ax25_list_lock vulnerable. It was not 100% verified if the real lockup
    can happen, but this fix isn't very costly and looks safe anyway.
    (It was tested by Bernard with 2.6.23.9 and 2.6.24-rc5 kernels.)

    Reported_by: Bernard Pidoux
    Tested_by: Bernard Pidoux
    Signed-off-by: Jarek Poplawski
    Signed-off-by: David S. Miller

    Jarek Poplawski
     

01 Nov, 2007

1 commit

  • Finally, the zero_it argument can be completely removed from
    the callers and from the function prototype.

    Besides, fix the checkpatch.pl warnings about using the
    assignments inside if-s.

    This patch is rather big, and it is a part of the previous one.
    I splitted it wishing to make the patches more readable. Hope
    this particular split helped.

    Signed-off-by: Pavel Emelyanov
    Signed-off-by: David S. Miller

    Pavel Emelyanov
     

20 Oct, 2007

1 commit

  • * Convert files to UTF-8.

    * Also correct some people's names
    (one example is Eißfeldt, which was found in a source file.
    Given that the author used an ß at all in a source file
    indicates that the real name has in fact a 'ß' and not an 'ss',
    which is commonly used as a substitute for 'ß' when limited to
    7bit.)

    * Correct town names (Goettingen -> Göttingen)

    * Update Eberhard Mönkeberg's address (http://lkml.org/lkml/2007/1/8/313)

    Signed-off-by: Jan Engelhardt
    Signed-off-by: Adrian Bunk

    Jan Engelhardt
     

11 Oct, 2007

6 commits

  • Since hardware header operations are part of the protocol class
    not the device instance, make them into a separate object and
    save memory.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: David S. Miller

    Stephen Hemminger
     
  • This patch makes most of the generic device layer network
    namespace safe. This patch makes dev_base_head a
    network namespace variable, and then it picks up
    a few associated variables. The functions:
    dev_getbyhwaddr
    dev_getfirsthwbytype
    dev_get_by_flags
    dev_get_by_name
    __dev_get_by_name
    dev_get_by_index
    __dev_get_by_index
    dev_ioctl
    dev_ethtool
    dev_load
    wireless_process_ioctl

    were modified to take a network namespace argument, and
    deal with it.

    vlan_ioctl_set and brioctl_set were modified so their
    hooks will receive a network namespace argument.

    So basically anthing in the core of the network stack that was
    affected to by the change of dev_base was modified to handle
    multiple network namespaces. The rest of the network stack was
    simply modified to explicitly use &init_net the initial network
    namespace. This can be fixed when those components of the network
    stack are modified to handle multiple network namespaces.

    For now the ifindex generator is left global.

    Fundametally ifindex numbers are per namespace, or else
    we will have corner case problems with migration when
    we get that far.

    At the same time there are assumptions in the network stack
    that the ifindex of a network device won't change. Making
    the ifindex number global seems a good compromise until
    the network stack can cope with ifindex changes when
    you change namespaces, and the like.

    Signed-off-by: Eric W. Biederman
    Signed-off-by: David S. Miller

    Eric W. Biederman
     
  • Every user of the network device notifiers is either a protocol
    stack or a pseudo device. If a protocol stack that does not have
    support for multiple network namespaces receives an event for a
    device that is not in the initial network namespace it quite possibly
    can get confused and do the wrong thing.

    To avoid problems until all of the protocol stacks are converted
    this patch modifies all netdev event handlers to ignore events on
    devices that are not in the initial network namespace.

    As the rest of the code is made network namespace aware these
    checks can be removed.

    Signed-off-by: Eric W. Biederman
    Signed-off-by: David S. Miller

    Eric W. Biederman
     
  • This patch modifies every packet receive function
    registered with dev_add_pack() to drop packets if they
    are not from the initial network namespace.

    This should ensure that the various network stacks do
    not receive packets in a anything but the initial network
    namespace until the code has been converted and is ready
    for them.

    Signed-off-by: Eric W. Biederman
    Signed-off-by: David S. Miller

    Eric W. Biederman
     
  • This patch passes in the namespace a new socket should be created in
    and has the socket code do the appropriate reference counting. By
    virtue of this all socket create methods are touched. In addition
    the socket create methods are modified so that they will fail if
    you attempt to create a socket in a non-default network namespace.

    Failing if we attempt to create a socket outside of the default
    network namespace ensures that as we incrementally make the network stack
    network namespace aware we will not export functionality that someone
    has not audited and made certain is network namespace safe.
    Allowing us to partially enable network namespaces before all of the
    exotic protocols are supported.

    Any protocol layers I have missed will fail to compile because I now
    pass an extra parameter into the socket creation code.

    [ Integrated AF_IUCV build fixes from Andrew Morton... -DaveM ]

    Signed-off-by: Eric W. Biederman
    Signed-off-by: David S. Miller

    Eric W. Biederman
     
  • This patch makes /proc/net per network namespace. It modifies the global
    variables proc_net and proc_net_stat to be per network namespace.
    The proc_net file helpers are modified to take a network namespace argument,
    and all of their callers are fixed to pass &init_net for that argument.
    This ensures that all of the /proc/net files are only visible and
    usable in the initial network namespace until the code behind them
    has been updated to be handle multiple network namespaces.

    Making /proc/net per namespace is necessary as at least some files
    in /proc/net depend upon the set of network devices which is per
    network namespace, and even more files in /proc/net have contents
    that are relevant to a single network namespace.

    Signed-off-by: Eric W. Biederman
    Signed-off-by: David S. Miller

    Eric W. Biederman
     

15 Aug, 2007

1 commit

  • commit 8d5cf596d10d740b69b5f4bbdb54b85abf75810d started to add statically
    allocated ax25_protocol's to list. However kfree() was still in place waiting
    for unsuspecting ones on module removal.

    Steps to reproduce:

    modprobe netrom
    rmmod netrom

    P.S.: code would benefit greatly from list_add/list_del usage

    kernel BUG at mm/slab.c:592!
    invalid opcode: 0000 [1] PREEMPT SMP
    CPU 0
    Modules linked in: netrom ax25 af_packet usbcore rtc_cmos rtc_core rtc_lib
    Pid: 4477, comm: rmmod Not tainted 2.6.23-rc3-bloat #2
    RIP: 0010:[] [] kfree+0x1c6/0x260
    RSP: 0000:ffff810079a05e48 EFLAGS: 00010046
    RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff81000000c000
    RDX: ffff81007e552458 RSI: 0000000000000000 RDI: 000000000000805d
    RBP: ffff810079a05e88 R08: 0000000000000001 R09: 0000000000000000
    R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff8805d080
    R13: ffffffff8805d080 R14: 0000000000000000 R15: 0000000000000282
    FS: 00002b73fc98aae0(0000) GS:ffffffff805dc000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
    CR2: 000000000053f3b8 CR3: 0000000079ff2000 CR4: 00000000000006e0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
    Process rmmod (pid: 4477, threadinfo ffff810079a04000, task ffff8100775aa480)
    Stack: ffff810079a05e68 0000000000000246 ffffffff8804eca0 0000000000000000
    ffffffff8805d080 00000000000000cf 0000000000000000 0000000000000880
    ffff810079a05eb8 ffffffff8803ec90 ffff810079a05eb8 0000000000000000
    Call Trace:
    [] :ax25:ax25_protocol_release+0xa0/0xb0
    [] :netrom:nr_exit+0x6b/0xf0
    [] sys_delete_module+0x170/0x1f0
    [] trace_hardirqs_on+0xd5/0x170
    [] trace_hardirqs_on_thunk+0x35/0x37
    [] system_call+0x7e/0x83

    Code: 0f 0b eb fe 66 66 90 66 66 90 48 8b 52 10 48 8b 02 25 00 40
    RIP [] kfree+0x1c6/0x260
    RSP
    Kernel panic - not syncing: Fatal exception

    Signed-off-by: Alexey Dobriyan
    Signed-off-by: David S. Miller

    Alexey Dobriyan
     

19 Jul, 2007

1 commit


11 Jul, 2007

1 commit


10 May, 2007

1 commit


09 May, 2007

1 commit


26 Apr, 2007

7 commits