12 Apr, 2014
1 commit
-
Several spots in the kernel perform a sequence like:
skb_queue_tail(&sk->s_receive_queue, skb);
sk->sk_data_ready(sk, skb->len);But at the moment we place the SKB onto the socket receive queue it
can be consumed and freed up. So this skb->len access is potentially
to freed up memory.Furthermore, the skb->len can be modified by the consumer so it is
possible that the value isn't accurate.And finally, no actual implementation of this callback actually uses
the length argument. And since nobody actually cared about it's
value, lots of call sites pass arbitrary values in such as '0' and
even '1'.So just remove the length argument from the callback, that way there
is no confusion whatsoever and all of these use-after-free cases get
fixed as a side effect.Based upon a patch by Eric Dumazet and his suggestion to audit this
issue tree-wide.Signed-off-by: David S. Miller
08 Apr, 2014
1 commit
-
net/tipc/socket.c: In function ‘tipc_release’:
net/tipc/socket.c:352: warning: ‘res’ is used uninitialized in this functionIntroduced by commit 24be34b5a0c9114541891d29dff1152bb1a8df34 ("tipc:
eliminate upcall function pointers between port and socket"), which
removed the sole initializer of "res".Just return 0 to fix it.
Signed-off-by: Geert Uytterhoeven
Signed-off-by: David S. Miller
04 Apr, 2014
1 commit
-
Commit 5902385a2440a55f005b266c93e0bb9398e5a62b ("tipc: obsolete
the remote management feature") introduces a regression where node
topology events are not being generated because the publication
that triggers this: {0, , } is no longer available.
This will break applications that rely on node events to discover
when nodes join/leave a cluster.We fix this by advertising the node publication when TIPC enters
networking mode, and withdraws it upon shutdown.Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Reviewed-by: Ying Xue
Signed-off-by: David S. Miller
29 Mar, 2014
2 commits
-
The node discovery domain is assigned when a bearer is enabled.
In the previous commit we reflect this attribute directly in the
bearer structure since it's needed to reinitialize the node
discovery mechanism after a hardware address change.There's no need to replicate this attribute anywhere else, so we
remove it from the tipc_link_req structure.Signed-off-by: Erik Hugne
Reviewed-by: Ying Xue
Signed-off-by: David S. Miller -
If the hardware address of a underlying netdevice is changed, it is
not enough to simply reset the bearer/links over this device. We
also need to reflect this change in the TIPC bearer and node
discovery structures aswell.This patch adds the necessary reinitialization of the node disovery
mechanism following a hardware address change so that the correct
originating media address is advertised in the discovery messages.Signed-off-by: Erik Hugne
Reported-by: Dong Liu
Reviewed-by: Ying Xue
Signed-off-by: David S. Miller
28 Mar, 2014
10 commits
-
Without properly implicit or explicit read memory barrier, it's
unsafe to read an atomic variable with atomic_read() from another
thread which is different with the thread of changing the atomic
variable with atomic_inc() or atomic_dec(). So a stale tipc_num_links
may be got with atomic_read() in tipc_node_get_links(). If the
tipc_num_links variable type is converted from atomic to unsigned
integer and node list lock is used to protect it, the issue would
be avoided.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
As tipc_node_list is protected by rcu read lock on read side, it's
unnecessary to hold node_list_lock to protect tipc_node_list in
tipc_node_get_links(). Instead, node_list_lock should just protects
tipc_num_nodes in the function.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
Convert tipc_node_list list and node_htable hash list to RCU lists.
On read side, the two lists are protected with RCU read lock, and
on update side, node_list_lock is applied to them.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
When a node is created, tipc_net_lock read lock is first held and
then node_create_lock is grabbed in order to prevent the same node
from being created and inserted into both node list and hlist twice.
But when we query node from the two node lists, we only hold
tipc_net_lock read lock without grabbing node_create_lock. Obviously
this locking policy is unable to guarantee that the two node lists
are always synchronized especially when the operation of changing
and accessing them occurs in different contexts like currently doing.Therefore, rename node_create_lock to node_list_lock to protect the
two node lists, that is, whenever node is inserted into them or node
is queried from them, the node_list_lock should be always held. As a
result, tipc_net_lock read lock becomes redundant and then can be
removed from the node query functions.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
Now unicast bearer is dynamically allocated and placed into its
identity specified slot of bearer_list array. When we search
bearer_list array with a bearer identity, the corresponding bearer
instance can be found. But broadcast bearer is statically allocated
and it is not located in the bearer_list array yet. So we decide to
enlarge bearer_list array into MAX_BEARERS + 1 slots, and its last
slot stores the broadcast bearer so that the broadcast bearer can
be found from bearer_list array with MAX_BEARERS as index. The
change will help us reduce the complex relationship between bearer
and link in the future.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
After the allocation of tipc_bearer structure instance is converted
from statical way to dynamical way, we identify whether a certain
tipc_bearer structure pointer is valid by checking whether the pointer
is NULL or not. So the active flag in tipc_bearer structure becomes
redundant.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
As part of the effort to introduce RCU protection for the bearer
list, we first need to change it to a list of pointers.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
The 'tipc_node_list' is guarded by tipc_net_lock and 'links' array
defined in 'tipc_node' structure is protected by node lock as well.
Without acquiring the two locks in named_cluster_distribute() a fatal
oops may happen in case that a destroyed link might be got and then
accessed. Therefore, above mentioned two locks must be held in
named_cluster_distribute() to prevent the issue from happening
accidentally.As 'links' array in node struct must be protected by node lock,
we have to move the code of selecting an active link from
tipc_link_xmit() to named_cluster_distribute() and then call
__tipc_link_xmit() with the selected link to deliver name messages.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
Due to the lacking of any credential, it's allowed to accept commands
requested from remote nodes to query the local node status, which is
prone to involve potential security risks. Instead, if we login to
a remote node with ssh command, this approach is not only more safe
than the remote management feature, but also it can give us more
permissions like changing the remote node configuration. So it's
reasonable for us to obsolete the remote management feature now.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
tipc_node_create routine doesn't need to check whether a node
object specified with a node address exists or not because its
caller(ie, tipc_disc_recv_msg routine) has checked this before
calling it.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller
26 Mar, 2014
1 commit
-
Conflicts:
Documentation/devicetree/bindings/net/micrel-ks8851.txt
net/core/netpoll.cThe net/core/netpoll.c conflict is a bug fix in 'net' happening
to code which is completely removed in 'net-next'.In micrel-ks8851.txt we simply have overlapping changes.
Signed-off-by: David S. Miller
25 Mar, 2014
1 commit
-
If a topology event subscription fails for any reason, such as out
of memory, max number reached or because we received an invalid
request the correct behavior is to terminate the subscribers
connection to the topology server. This is currently broken and
produces the following oops:[27.953662] tipc: Subscription rejected, illegal request
[27.955329] BUG: spinlock recursion on CPU#1, kworker/u4:0/6
[27.957066] lock: 0xffff88003c67f408, .magic: dead4ead, .owner: kworker/u4:0/6, .owner_cpu: 1
[27.958054] CPU: 1 PID: 6 Comm: kworker/u4:0 Not tainted 3.14.0-rc6+ #5
[27.960230] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[27.960874] Workqueue: tipc_rcv tipc_recv_work [tipc]
[27.961430] ffff88003c67f408 ffff88003de27c18 ffffffff815c0207 ffff88003de1c050
[27.962292] ffff88003de27c38 ffffffff815beec5 ffff88003c67f408 ffffffff817f0a8a
[27.963152] ffff88003de27c58 ffffffff815beeeb ffff88003c67f408 ffffffffa0013520
[27.964023] Call Trace:
[27.964292] [] dump_stack+0x45/0x56
[27.964874] [] spin_dump+0x8c/0x91
[27.965420] [] spin_bug+0x21/0x26
[27.965995] [] do_raw_spin_lock+0x116/0x140
[27.966631] [] _raw_spin_lock_bh+0x15/0x20
[27.967256] [] subscr_conn_shutdown_event+0x20/0xa0 [tipc]
[27.968051] [] tipc_close_conn+0xa4/0xb0 [tipc]
[27.968722] [] tipc_conn_terminate+0x1a/0x30 [tipc]
[27.969436] [] subscr_conn_msg_event+0x1f2/0x2f0 [tipc]
[27.970209] [] tipc_receive_from_sock+0x90/0xf0 [tipc]
[27.970972] [] tipc_recv_work+0x29/0x50 [tipc]
[27.971633] [] process_one_work+0x165/0x3e0
[27.972267] [] worker_thread+0x119/0x3a0
[27.972896] [] ? manage_workers.isra.25+0x2a0/0x2a0
[27.973622] [] kthread+0xdf/0x100
[27.974168] [] ? kthread_create_on_node+0x1a0/0x1a0
[27.974893] [] ret_from_fork+0x7c/0xb0
[27.975466] [] ? kthread_create_on_node+0x1a0/0x1a0The recursion occurs when subscr_terminate tries to grab the
subscriber lock, which is already taken by subscr_conn_msg_event.
We fix this by checking if the request to establish a new
subscription was successful, and if not we initiate termination of
the subscriber after we have released the subscriber lock.Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller
15 Mar, 2014
1 commit
-
Conflicts:
drivers/net/usb/r8152.c
drivers/net/xen-netback/netback.cBoth the r8152 and netback conflicts were simple overlapping
changes.Signed-off-by: David S. Miller
13 Mar, 2014
8 commits
-
As an artefact from the native interface, the message sending functions
in the port takes a port ref as first parameter, and then looks up in
the registry to find the corresponding port pointer. This despite the
fact that the only currently existing caller, tipc_sock, already knows
this pointer.We change the signature of these functions to take a struct tipc_port*
argument, and remove the redundant lookups.We also remove an unmotivated extra lookup in the function
socket.c:auto_connect(), and, as the lookup functions tipc_port_deref()
and ref_deref() now become unused, we remove these two functions.Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
The practice of naming variables in TIPC is inconistent, sometimes
even within the same file.In this commit we align variable names and declarations within
socket.c, and function and macro names within socket.h. We also
reduce the number of conversion macros to two, in order to make
usage less obsure.These changes are purely cosmetic.
Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
The three functions tipc_portimportance(), tipc_portunreliable() and
tipc_portunreturnable() and their corresponding tipc_set* functions,
are all grabbing port_lock when accessing the targeted port. This is
unnecessary in the current code, since these calls only are made from
within socket downcalls, already protected by sock_lock.We remove the redundant locking. Also, since the functions now become
trivial one-liners, we move them to port.h and make them inline.Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
Due to the original one-to-many relation between port and user API
layers, upcalls to the API have been performed via function pointers,
installed in struct tipc_port at creation. Since this relation now
always is one-to-one, we can instead use ordinary function calls.We remove the function pointers 'dispatcher' and ´wakeup' from
struct tipc_port, and replace them with calls to the renamed
functions tipc_sk_rcv() and tipc_sk_wakeup().At the same time we change the name and signature of the functions
tipc_createport() and tipc_deleteport() to reflect their new role
as mere initialization/destruction functions.Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
After the removal of the tipc native API the relation between
a tipc_port and its API types is strictly one-to-one, i.e, the
latter can now only be a socket API. There is therefore no need
to allocate struct tipc_port and struct sock independently.In this commit, we aggregate struct tipc_port into struct tipc_sock,
hence saving both CPU cycles and structure complexity.There are no functional changes in this commit, except for the
elimination of the separate allocation/freeing of tipc_port.
All other changes are just adaptatons to the new data structure.This commit also opens up for further code simplifications and
code volume reduction, something we will do in later commits.Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
The field 'peer_name' in struct tipc_sock is redundant, since
this information already is available from tipc_port, to which
tipc_sock has a reference.We remove the field, and ensure that peer node and peer port
info instead is fetched via the functions that already exist
for this purpose.Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
The lock for protecting the reference table is declared as an
RWLOCK, although it is only used in write mode, never in read
mode.We redefine it to become a spinlock.
Signed-off-by: Jon Maloy
Reviewed-by: Ying Xue
Reviewed-by: Erik Hugne
Signed-off-by: David S. Miller -
The use of __constant_ has been unnecessary for quite awhile now.
Make these uses consistent with the rest of the kernel.
Signed-off-by: Joe Perches
Signed-off-by: David S. Miller
07 Mar, 2014
6 commits
-
Failure to schedule a TIPC tasklet with tipc_k_signal because the
tasklet handler is disabled is not an error. It means TIPC is
currently in the process of shutting down. We remove the error
logging in this case.Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
When the TIPC module is removed, the tasklet handler is disabled
before all other subsystems. This will cause lingering publications
in the name table because the node_down tasklets responsible to
clean up publications from an unreachable node will never run.
When the name table is shut down, these publications are detected
and an error message is logged:
tipc: nametbl_stop(): orphaned hash chain detected
This is actually a memory leak, introduced with commit
993b858e37b3120ee76d9957a901cca22312ffaa ("tipc: correct the order
of stopping services at rmmod")Instead of just logging an error and leaking memory, we free
the orphaned entries during nametable shutdown.Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
When a topology server subscriber is disconnected, the associated
connection id is set to zero. A check vs zero is then done in the
subscription timeout function to see if the subscriber have been
shut down. This is unnecessary, because all subscription timers
will be cancelled when a subscriber terminates. Setting the
connection id to zero is actually harmful because id zero is the
identity of the topology server listening socket, and can cause a
race that leads to this socket being closed instead.Signed-off-by: Erik Hugne
Acked-by: Ying Xue
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
When messages are received via tipc socket under non-block mode,
schedule_timeout() is called in tipc_wait_for_rcvmsg(), that is,
the process of receiving messages will be scheduled once although
timeout value passed to schedule_timeout() is 0. The same issue
exists in accept()/wait_for_accept(). To avoid this unnecessary
process switch, we only call schedule_timeout() if the timeout
value is non-zero.Signed-off-by: Ying Xue
Reviewed-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
When tipc_conn_sendmsg() calls tipc_conn_lookup() to query a
connection instance, its reference count value is increased if
it's found. But subsequently if it's found that the connection is
closed, the work of sending message is not queued into its server
send workqueue, and the connection reference count is not decreased.
This will cause a reference count leak. To reproduce this problem,
an application would need to open and closes topology server
connections with high intensity.We fix this by immediately decrementing the connection reference
count if a send fails due to the connection being closed.Signed-off-by: Ying Xue
Acked-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller -
Currently connection shutdown callback function is called when
connection instance is released in tipc_conn_kref_release(), and
receiving packets and sending packets are running in different
threads. Even if connection is closed by the thread of receiving
packets, its shutdown callback may not be called immediately as
the connection reference count is non-zero at that moment. So,
although the connection is shut down by the thread of receiving
packets, the thread of sending packets doesn't know it. Before
its shutdown callback is invoked to tell the sending thread its
connection has been closed, the sending thread may deliver
messages by tipc_conn_sendmsg(), this is why the following error
information appears:"Sending subscription event failed, no memory"
To eliminate it, allow connection shutdown callback function to
be called before connection id is removed in tipc_close_conn(),
which makes the sending thread know the truth in time that its
socket is closed so that it doesn't send message to it. We also
remove the "Sending XXX failed..." error reporting for topology
and config services.Signed-off-by: Ying Xue
Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller
06 Mar, 2014
1 commit
-
Conflicts:
drivers/net/wireless/ath/ath9k/recv.c
drivers/net/wireless/mwifiex/pcie.c
net/ipv6/sit.cThe SIT driver conflict consists of a bug fix being done by hand
in 'net' (missing u64_stats_init()) whilst in 'net-next' a helper
was created (netdev_alloc_pcpu_stats()) which takes care of this.The two wireless conflicts were overlapping changes.
Signed-off-by: David S. Miller
22 Feb, 2014
2 commits
-
Accidentally a side effect is involved by commit 6e967adf7(tipc:
relocate common functions from media to bearer). Now tipc stack
handler of receiving packets from netdevices as well as netdevice
notification handler are registered when bearer is enabled rather
than tipc module initialization stage, but the two handlers are
both unregistered in tipc module exit phase. If tipc module is
inserted and then immediately removed, the following warning
message will appear:"dev_remove_pack: ffffffffa0380940 not found"
This is because in module insertion stage tipc stack packet handler
is not registered at all, but in module exit phase dev_remove_pack()
needs to remove it. Of course, dev_remove_pack() cannot find tipc
protocol handler from the kernel protocol handler list so that the
warning message is printed out.But if registering the two handlers is adjusted from enabling bearer
phase into inserting module stage, the warning message will be
eliminated. Due to this change, tipc_core_start_net() and
tipc_core_stop_net() can be deleted as well.Reported-by: Wang Weidong
Cc: Jon Maloy
Cc: Erik Hugne
Signed-off-by: Ying Xue
Reviewed-by: Paul Gortmaker
Signed-off-by: David S. Miller -
When tipc module is inserted, many tipc components are initialized
one by one. During the initialization period, if one of them is
failed, tipc_core_stop() will be called to stop all components
whatever corresponding components are created or not. To avoid to
release uncreated ones, relevant components have to add necessary
enabled flags indicating whether they are created or not.But in the initialization stage, if one component is unsuccessfully
created, we will just destroy successfully created components before
the failed component instead of all components. All enabled flags
defined in components, in turn, become redundant. Additionally it's
also unnecessary to identify whether table.types is NULL in
tipc_nametbl_stop() because name stable has been definitely created
successfully when tipc_nametbl_stop() is called.Cc: Jon Maloy
Cc: Erik Hugne
Signed-off-by: Ying Xue
Reviewed-by: Paul Gortmaker
Signed-off-by: David S. Miller
20 Feb, 2014
1 commit
-
When a message could not be sent out because the destination node
or link could not be found, the full message size is returned from
sendmsg() as if it had been sent successfully. An application will
then get a false indication that it's making forward progress. This
problem has existed since the initial commit in 2.6.16.We change this to return -ENETUNREACH if the message cannot be
delivered due to the destination node/link being unavailable. We
also get rid of the redundant tipc_reject_msg call since freeing
the buffer and doing a tipc_port_iovec_reject accomplishes exactly
the same thing.Signed-off-by: Erik Hugne
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller
19 Feb, 2014
2 commits
-
Conflicts:
drivers/net/bonding/bond_3ad.h
drivers/net/bonding/bond_main.cTwo minor conflicts in bonding, both of which were overlapping
changes.Signed-off-by: David S. Miller
-
Rename the following functions, which are shorter and more in line
with common naming practice in the network subsystem.tipc_bclink_send_msg->tipc_bclink_xmit
tipc_bclink_recv_pkt->tipc_bclink_rcv
tipc_disc_recv_msg->tipc_disc_rcv
tipc_link_send_proto_msg->tipc_link_proto_xmit
link_recv_proto_msg->tipc_link_proto_rcv
link_send_sections_long->tipc_link_iovec_long_xmit
tipc_link_send_sections_fast->tipc_link_iovec_xmit_fast
tipc_link_send_sync->tipc_link_sync_xmit
tipc_link_recv_sync->tipc_link_sync_rcv
tipc_link_send_buf->__tipc_link_xmit
tipc_link_send->tipc_link_xmit
tipc_link_send_names->tipc_link_names_xmit
tipc_named_recv->tipc_named_rcv
tipc_link_recv_bundle->tipc_link_bundle_rcv
tipc_link_dup_send_queue->tipc_link_dup_queue_xmit
link_send_long_buf->tipc_link_frag_xmittipc_multicast->tipc_port_mcast_xmit
tipc_port_recv_mcast->tipc_port_mcast_rcv
tipc_port_reject_sections->tipc_port_iovec_reject
tipc_port_recv_proto_msg->tipc_port_proto_rcv
tipc_connect->tipc_port_connect
__tipc_connect->__tipc_port_connect
__tipc_disconnect->__tipc_port_disconnect
tipc_disconnect->tipc_port_disconnect
tipc_shutdown->tipc_port_shutdown
tipc_port_recv_msg->tipc_port_rcv
tipc_port_recv_sections->tipc_port_iovec_rcvrelease->tipc_release
accept->tipc_accept
bind->tipc_bind
get_name->tipc_getname
poll->tipc_poll
send_msg->tipc_sendmsg
send_packet->tipc_send_packet
send_stream->tipc_send_stream
recv_msg->tipc_recvmsg
recv_stream->tipc_recv_stream
connect->tipc_connect
listen->tipc_listen
shutdown->tipc_shutdown
setsockopt->tipc_setsockopt
getsockopt->tipc_getsockoptAbove changes have no impact on current users of the functions.
Signed-off-by: Ying Xue
Reviewed-by: Jon Maloy
Signed-off-by: David S. Miller
17 Feb, 2014
2 commits
-
I commit e099e86c9e24fe9aff36773600543eb31d8954d
("tipc: add node_lock protection to link lookup function")
we are calling spin_lock(&node->lock) directly instead of indirectly
via the tipc_node_lock(node) function. However, tipc_node_lock() is
using spin_lock_bh(), not spin_lock(), something leading to
unbalanced usage in one place, and a smatch warning.We fix this by consistently using tipc_node_lock()/unlock() in
in the places touched by the mentioned commit.Signed-off-by: Jon Maloy
Signed-off-by: David S. Miller -
In commit 7d33939f475d403e79124e3143d7951dcfe8629f
("tipc: delay delete of link when failover is needed") we
introduced a loop for finding and removing a link pointer
in an array. The removal is done after we have left the loop,
giving the impression that one may remove the wrong pointer
if no matching element is found.This is not really a bug, since we know that there will always
be a matching element, but it looks wrong, and causes a smatch
warning.We fix this loop with this commit.
Signed-off-by: Jon Maloy
Signed-off-by: David S. Miller