netdev - Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err

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, 18 Sep 2013 20:00:19 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	David Miller <davem@...emloft.net>
Cc:	duanj.fnst@...fujitsu.com, netdev@...r.kernel.org
Subject: Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

On Wed, Sep 18, 2013 at 12:36:13PM -0400, David Miller wrote:
> From: Duan Jiong <duanj.fnst@...fujitsu.com>
> Date: Wed, 18 Sep 2013 20:04:48 +0800
> 
> > From: Duan Jiong <duanj.fnst@...fujitsu.com>
> > 
> > when dealing with redirect message in udpv6_err() and
> > rawv6_err() the err shoud be assigned to 0, not EPROTO.
> > 
> > Signed-off-by: Duan Jiong <duanj.fnst@...fujitsu.com>
> 
> No, you should fix this the same way you handled DCCP, but skipping
> the whole socket error setting path for NDISC_REDIRECT.
> 
> Clearing the socket error to zero is not correct, it means you will
> lose any existing error setting.

My reasoning when I proposed this change was that we should not stop
appending ipv6 redirects to the raw and udp sockets error queues because
userland could want to deal with them (and probably already does). Also,
we need to notify poll of the appended error, hence we still needed a
call to sk_err_report to fire up a POLL_ERR event. Because we don't know
if the router sending the redirect actually dropped the packet I thought
taht '0' would be the only sane choice. We currently report back EPROTO,
IPv4 does report (rightfully) EHOSTUNREACH.

Perhaps we should stay with EPROTO?

I agree, all other error functions should return early after updating
the routing table without reporting any error. Either they should not
call into this function or ignore err == 0 (I prefer the first).

I know, it is a bit hacky but this was the most sane idea I came up with.

(I still have not reviewed this patch, just wanted to provide a bit of
background that lead to this change.)

Greetings,

  Hannes

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