[PATCH net-next] rtnetlink: enable alt_ifname for setlink/newlink

[PATCH net-next] rtnetlink: enable alt_ifname for setlink/newlink

[Florent Fourcot]

buffer is always valid when called by setlink/newlink,
but contains only empty string when IFLA_IFNAME is not given. So
IFLA_ALT_IFNAME is always ignored

Fixes: 76c9ac0ee878 ("net: rtnetlink: add possibility to use alternative names as message handle")
Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
Signed-off-by: Brian Baboch <brian.baboch@wifirst.fr>
---
 net/core/rtnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3313419bbcba..613065a53b34 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2979,7 +2979,7 @@ static struct net_device *rtnl_dev_get(struct net *net,
 {
 	char buffer[ALTIFNAMSIZ];
 
-	if (!ifname) {
+	if (!ifname || !ifname[0]) {
 		ifname = buffer;
 		if (ifname_attr)
 			nla_strscpy(ifname, ifname_attr, IFNAMSIZ);
-- 
2.30.2

Re: [PATCH net-next] rtnetlink: enable alt_ifname for setlink/newlink

[Jakub Kicinski]

On Thu, 31 Mar 2022 14:37:28 +0200 Florent Fourcot wrote:
> buffer is always valid when called by setlink/newlink,
> but contains only empty string when IFLA_IFNAME is not given. So
> IFLA_ALT_IFNAME is always ignored
> 
> Fixes: 76c9ac0ee878 ("net: rtnetlink: add possibility to use alternative names as message handle")

Again, you most definitely need to CC the author of the code under
Fixes, they may have some context we don't.

Since this is a fix the subject tag should be [PATCH net].

> Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
> Signed-off-by: Brian Baboch <brian.baboch@wifirst.fr>
> ---
>  net/core/rtnetlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 3313419bbcba..613065a53b34 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2979,7 +2979,7 @@ static struct net_device *rtnl_dev_get(struct net *net,
>  {
>  	char buffer[ALTIFNAMSIZ];
>  
> -	if (!ifname) {
> +	if (!ifname || !ifname[0]) {

Unless I'm missing something this function is loading a footgun to 
save copying 16B twice. I'd remove the ifname argument, it's always
populated from ifname_attr by the callers AFAICT.