linux-kernel - Re: net: fix routing encapsulated packets when binding a socket to a tunnel interface

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:   Mon, 01 Apr 2019 11:00:22 +0800
From:   lifonghsu <lifonghsu@...ology.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: net: fix routing encapsulated packets when binding a socket to a
 tunnel interface

David Miller 於 2019-03-30 01:40 寫到:
> From: lifonghsu <lifonghsu@...ology.com>
> Date: Thu, 28 Mar 2019 16:30:37 +0800
> 
>> Indeed, skb_iif is used as receive site indication to present "device
>> the packet arrived on".
>> This commit keeps the previous arrived device (similar to the concept
>> of "device the packet arrived on") in skb_iif field to prevent kernel
>> from referring sk_bound_dev_if again. Otherwise, we might need to add
>> a new field to sk_buff structure for our purpose.
> 
> Therefore, you are deciding to arbitrarily repurpose an RX side piece
> of state for TX purposes.
> 
> Do not do this.
> 
> It confuses anyone trying to understand how skb_iif works.
> 
> You must use something with a different name, and clear semantics, to
> achieve this goal.
> 
> For example, you could use an anonymous union:
> 
> 	union {
> 		int	skb_iif;
> 		bool	bound_dev_already_applied;
> 	};
> 
> You never actually _USE_ the value of skb_iif, it is just merely a
> boolean indicating whether sk_bound_dev_if was applied already.

Since we are not sure whether there is any code referring to the value 
of
skb_iff in transmit site or not, it is risky to set an invalid interface
index to skb_iif in an anonymous union.
What if we add a bit (e.g., __u8 bound_dev_already_applied:1;) to 
sk_buff?