Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lxc_user_nic: fix get_mtu() error handling #4252

Merged
merged 1 commit into from Jan 6, 2023

Conversation

mihalicyn
Copy link
Member

get_mtu() returns int, but "mtu" variable has unsigned int type. It leads to logical error in error handling, which can end up with strange -EINVAL error in lxc_veth_create(), cause (mtu > 0) condition is met, but negative "mtu" value is too large when set as mtu for network device.

Issue #4232

Signed-off-by: Alexander Mikhalitsyn aleksandr.mikhalitsyn@canonical.com

Copy link
Member

@brauner brauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe moving all of this into get_mtu() is nicer?

diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c
index a91e2259d..3a015779f 100644
--- a/src/lxc/cmd/lxc_user_nic.c
+++ b/src/lxc/cmd/lxc_user_nic.c
@@ -489,20 +489,26 @@ static int instantiate_veth(char *veth1, char *veth2, pid_t pid, unsigned int mt
        return netdev_set_flag(veth1, IFF_UP);
 }

-static int get_mtu(char *name)
+#define NETDEV_MTU_DEFAULT 1500
+
+static unsigned int get_mtu(const char *name)
 {
-       int idx;
+       int idx, val;

        idx = if_nametoindex(name);
        if (idx < 0)
-               return -1;
+               return NETDEV_MTU_DEFAULT;
+
+       val = netdev_get_mtu(idx);
+       if (val < 0)
+               return NETDEV_MTU_DEFAULT;

-       return netdev_get_mtu(idx);
+       return val;
 }

 static int create_nic(char *nic, char *br, int pid, char **cnic)
 {
-       unsigned int mtu = 1500;
+       unsigned int mtu = NETDEV_MTU_DEFAULT;
        int ret;
        char veth1buf[IFNAMSIZ], veth2buf[IFNAMSIZ];

@@ -520,8 +526,6 @@ static int create_nic(char *nic, char *br, int pid, char **cnic)

        if (strcmp(br, "none"))
                mtu = get_mtu(br);
-       if (!mtu)
-               mtu = 1500;

        /* create the nics */
        ret = instantiate_veth(veth1buf, veth2buf, pid, mtu);

@lxc-jenkins
Copy link

Testsuite passed

@mihalicyn
Copy link
Member Author

Thanks, Christian! Let's take this way :)

get_mtu() returns int, but "mtu" variable has unsigned int type.
It leads to logical error in error handling, which can end up
with strange -EINVAL error in lxc_veth_create(), cause (mtu > 0)
condition is met, but negative "mtu" value is too large when set
as mtu for network device.

Issue lxc#4232

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
@mihalicyn
Copy link
Member Author

Have done. I've also added error messages for convenience.

@lxc-jenkins
Copy link

Testsuite passed

@brauner brauner merged commit 2097da8 into lxc:master Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants