netdev - Re: ipx: fix locking regression in ipx_sendmsg and ipx

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 19 Nov 2014 11:34:13 +0100
From:	Jiri Bohac <jbohac@...e.cz>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Jiri Bohac <jbohac@...e.cz>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg

On Wed, Nov 19, 2014 at 09:32:54AM +0100, Arnd Bergmann wrote:
> I'm more interested in the code structure, in particular this bit
> 
> @@ -1807,8 +1812,10 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>         rc = skb_copy_datagram_iovec(skb, sizeof(struct ipxhdr), msg->msg_iov,
>                                      copied);
> -       if (rc)
> -               goto out_free;
> +       if (rc) {
> +               skb_free_datagram(sk, skb);
> +               goto out;
> +       }
> 
> 
> after your change mixes coding styles: in some failure cases you just goto
> a cleanup part, in other cases you do the cleanup before the goto.
> 
> If I'm reading the patch correctly, this change has introduced a leak

Very lame of me - thanks so much for spotitng this!
Sending a restructured v3.

-- 
Jiri Bohac <jbohac@...e.cz>
SUSE Labs, SUSE CZ

--
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