| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4755E01B.9010307@gmail.com>
Date: Wed, 05 Dec 2007 00:17:47 +0100
From: Jarek Poplawski <jarkao2@...il.com>
To: Bernard Pidoux <pidoux@....jussieu.fr>
CC: Jarek Poplawski <jarkao2@...pl>, ralf@...ux-mips.org,
davem@...emloft.net, netdev@...r.kernel.org,
Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: Inconsistent lock state and possible irq lock inversion dependency
detected in ax25.ko
Bernard Pidoux wrote, On 12/04/2007 11:26 PM:
> Jarek Poplawski wrote:
>> Bernard Pidoux wrote, On 12/02/2007 06:37 PM:
>>
>>> Hi,
>>>
>>> Many thanks for your patch for ~/net/ax25/ax25_subr.c
>>>
>>> Introduction of local_bh_disable() ... local_bh_enable()
>>>
>>> cured the inconsistent lock state related to AX25 connect timeout.
>>>
>>> I have now a stable monoprocessor system running AX25 and ROSE network
>>> packet switching application FPAC, whether kernel is compiled with or
>>> without hack option.
>>>
>>> There is no more problem during normal operations.
>>>
>>> This was achieved, thanks to your AX25 patch and the patch from Alexey
>>> Dobriyan for rose module.
>>>
>>> I also patched rose module in order to get packet routing more
>>> efficient, taking into account the "restarted" flag that is raised when
>>> a neighbour node is already connected.
>>>
>>> To summarize the present situation on my Linux machine, I built a patch
>>> against kernel 2.6.23.9.
>>>
>>> I would appreciate if you could make it included into a next kernel release.
>> ...
>>
>> Bernard, I'm very glad I could be a little helpful, but I'm not sure of
>> your intentions: my patch proposal is rather trivial interpretation of
>> lockdep's report; I haven't studied AX25 enough even to be sure there is
>> a real lockup possible in this place. Since this change looks not very
>> costly and quite safe, I can 'take a risk' to sign this off after your
>> testing. But anything more is beyond my 'range'.
>>
>> So, since you've spent quite a lot of time on this all, maybe it would
>> be simpler if you've tried the same with the current kernel, and resent
>> "proper" (not gzipped and with changelog) patch or patches. Then, I hope,
>> Ralf, as the maintainer, will make the rest.
>>
>> Regards,
>> Jarek P.
>>
>>
>
> As required I send again in clear text the summary of ax25 and rose
> patch against 2.6.23.9.
> As I said, the kernel stability, after applying these patch, is behind us.
> I did not observe any warning nor lockup after a few days of AX25
> intensive use.
> Also rose module is handling routing of frames much more efficiently.
>
> This will considerably help us to focus on application programs now.
> I am now concentrating my efforts on ROSE/FPAC and Linux FBB code
> adjustement.
>
> Thanks to you all for your help.
>
> 73 de Bernard, f6bvp
>
>
>
Bernard, I think you've forgotten at least about some distinct
changelog and signed-off-by?
Jarek P.
---
ax25_subr.c part:
Acked-by: Jarek Poplawski <jarkao2@...il.com>
> ------------------------------------------------------------------------
>
> diff -pruN a/include/net/rose.h b/include/net/rose.h
> --- a/include/net/rose.h 2007-10-12 18:43:44.000000000 +0200
> +++ b/include/net/rose.h 2007-12-01 23:56:57.000000000 +0100
> @@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first
> extern struct net_device *rose_dev_get(rose_address *);
> extern struct rose_route *rose_route_free_lci(unsigned int, struct rose_neigh *);
> extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
> +extern struct rose_neigh *__rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
> extern int rose_rt_ioctl(unsigned int, void __user *);
> extern void rose_link_failed(ax25_cb *, int);
> extern int rose_route_frame(struct sk_buff *, ax25_cb *);
> diff -pruN a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
> --- a/net/ax25/ax25_subr.c 2007-10-12 18:43:44.000000000 +0200
> +++ b/net/ax25/ax25_subr.c 2007-12-01 23:32:01.000000000 +0100
> @@ -279,6 +279,7 @@ void ax25_disconnect(ax25_cb *ax25, int
> ax25_link_failed(ax25, reason);
>
> if (ax25->sk != NULL) {
> + local_bh_disable();
> bh_lock_sock(ax25->sk);
> ax25->sk->sk_state = TCP_CLOSE;
> ax25->sk->sk_err = reason;
> @@ -288,5 +289,6 @@ void ax25_disconnect(ax25_cb *ax25, int
> sock_set_flag(ax25->sk, SOCK_DEAD);
> }
> bh_unlock_sock(ax25->sk);
> + local_bh_enable();
> }
> }
> diff -pruN a/net/rose/af_rose.c b/net/rose/af_rose.c
> --- a/net/rose/af_rose.c 2007-10-12 18:43:44.000000000 +0200
> +++ b/net/rose/af_rose.c 2007-12-02 10:06:31.000000000 +0100
> @@ -62,7 +62,7 @@ int sysctl_rose_window_size
> static HLIST_HEAD(rose_list);
> static DEFINE_SPINLOCK(rose_list_lock);
>
> -static struct proto_ops rose_proto_ops;
> +static const struct proto_ops rose_proto_ops;
>
> ax25_address rose_callsign;
>
> @@ -741,7 +741,7 @@ static int rose_connect(struct socket *s
> sk->sk_state = TCP_CLOSE;
> sock->state = SS_UNCONNECTED;
>
> - rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
> + rose->neighbour = __rose_get_neigh(&addr->srose_addr, &cause,
> &diagnostic);
> if (!rose->neighbour)
> return -ENETUNREACH;
> @@ -773,7 +773,6 @@ static int rose_connect(struct socket *s
>
> rose_insert_socket(sk); /* Finish the bind */
> }
> -rose_try_next_neigh:
> rose->dest_addr = addr->srose_addr;
> rose->dest_call = addr->srose_call;
> rose->rand = ((long)rose & 0xFFFF) + rose->lci;
> @@ -835,12 +834,6 @@ rose_try_next_neigh:
> }
>
> if (sk->sk_state != TCP_ESTABLISHED) {
> - /* Try next neighbour */
> - rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic);
> - if (rose->neighbour)
> - goto rose_try_next_neigh;
> -
> - /* No more neighbours */
> sock->state = SS_UNCONNECTED;
> err = sock_error(sk); /* Always set at this point */
> goto out_release;
> @@ -1481,7 +1474,7 @@ static struct net_proto_family rose_fami
> .owner = THIS_MODULE,
> };
>
> -static struct proto_ops rose_proto_ops = {
> +static const struct proto_ops rose_proto_ops = {
> .family = PF_ROSE,
> .owner = THIS_MODULE,
> .release = rose_release,
> Les fichiers linux-2.6.23.9-orig/net/rose/rose.ko et linux-2.6.23.9/net/rose/rose.ko sont différents.
> diff -pruN a/net/rose/rose_route.c b/net/rose/rose_route.c
> --- a/net/rose/rose_route.c 2007-10-12 18:43:44.000000000 +0200
> +++ b/net/rose/rose_route.c 2007-12-02 00:15:24.000000000 +0100
> @@ -664,25 +664,22 @@ struct rose_route *rose_route_free_lci(u
> /*
> * Find a neighbour given a ROSE address.
> */
> -struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
> +struct rose_neigh *__rose_get_neigh(rose_address *addr, unsigned char *cause,
> unsigned char *diagnostic)
> {
> - struct rose_neigh *res = NULL;
> struct rose_node *node;
> int failed = 0;
> int i;
>
> - spin_lock_bh(&rose_node_list_lock);
> for (node = rose_node_list; node != NULL; node = node->next) {
> if (rosecmpm(addr, &node->address, node->mask) == 0) {
> for (i = 0; i < node->count; i++) {
> - if (!rose_ftimer_running(node->neighbour[i])) {
> - res = node->neighbour[i];
> - goto out;
> - } else
> - failed = 1;
> + if (node->neighbour[i]->restarted)
> + return node->neighbour[i];
> + if (!rose_ftimer_running(node->neighbour[i]))
> + return node->neighbour[i];
> + failed = 1;
> }
> - break;
> }
> }
>
> @@ -694,7 +691,16 @@ struct rose_neigh *rose_get_neigh(rose_a
> *diagnostic = 0;
> }
>
> -out:
> + return NULL;
> +}
> +
> +struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
> + unsigned char *diagnostic)
> +{
> + struct rose_neigh *res;
> +
> + spin_lock_bh(&rose_node_list_lock);
> + res = __rose_get_neigh(addr, cause, diagnostic);
> spin_unlock_bh(&rose_node_list_lock);
>
> return res;
> @@ -1019,7 +1025,7 @@ int rose_route_frame(struct sk_buff *skb
> rose_route = rose_route->next;
> }
>
> - if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
> + if ((new_neigh = __rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
> rose_transmit_clear_request(rose_neigh, lci, cause, diagnostic);
> goto out;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists