Quoting S.Çağlar Onur ([email protected]): > Hey Serge, > > On Mon, Feb 17, 2014 at 6:03 PM, Serge Hallyn <[email protected]> wrote: > > Quoting S.Çağlar Onur ([email protected]): > >> Signed-off-by: S.Çağlar Onur <[email protected]> > > > > Thanks! One comment, > > > >> --- > >> src/lxc/conf.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/src/lxc/conf.c b/src/lxc/conf.c > >> index 10f46ae..175a82f 100644 > >> --- a/src/lxc/conf.c > >> +++ b/src/lxc/conf.c > >> @@ -3011,13 +3011,14 @@ void lxc_delete_network(struct lxc_handler > >> *handler) > >> > >> #define LXC_USERNIC_PATH LIBEXECDIR "/lxc/lxc-user-nic" > >> > >> +/* lxc-user-nic returns "interface_name:interface_name\n" */ > >> +#define MAX_BUFFER_SIZE IFNAMSIZ*2 + 2 > >> static int unpriv_assign_nic(struct lxc_netdev *netdev, pid_t pid) > >> { > >> pid_t child; > >> int bytes, pipefd[2]; > >> char *token, *saveptr = NULL; > >> - /* lxc-user-nic returns "interface_name:interface_name" format */ > >> - char buffer[IFNAMSIZ*2 + 1]; > >> + char buffer[MAX_BUFFER_SIZE]; > >> > >> if (netdev->type != LXC_NET_VETH) { > >> ERROR("nic type %d not support for unprivileged use", > >> @@ -3043,7 +3044,7 @@ static int unpriv_assign_nic(struct lxc_netdev > >> *netdev, pid_t pid) > >> /* redirect the stdout to write-end of the pipe */ > >> dup2(pipefd[1], STDOUT_FILENO); > >> /* close the write-end of the pipe */ > >> - close(pipefd[0]); > >> + close(pipefd[1]); > >> > >> // Call lxc-user-nic pid type bridge > >> char pidstr[20]; > >> @@ -3058,7 +3059,7 @@ static int unpriv_assign_nic(struct lxc_netdev > >> *netdev, pid_t pid) > >> /* close the write-end of the pipe */ > >> close(pipefd[1]); > >> > >> - bytes = read(pipefd[0], &buffer, IFNAMSIZ*2 + 1); > >> + bytes = read(pipefd[0], &buffer, MAX_BUFFER_SIZE); > > > > Now technically this could mess us up, since the last byte we > > read may not be 0. > > > > So for the read call itself I think it would be better to > > stick to IFNAMSIZ*2 + 1). Am I thinking clearly? > > I think (if I'm not missing something obvious) we are reading up to > MAX_BUFFER_SIZE (which is "IFNAMSIZ:IFNAMSIZ\n") and then placing \0 > to end of the the buffer via next line (which is not shown here but > looks like this buffer[bytes - 1] = '\0';). Do you see a problem > there?
Sorry - you're right. It didnt' show up in yoru context and I thought the buf was being memset earlier rather than len-1 being set to 0 at end. It's already been applied, but in any case Acked-by: Serge E. Hallyn <[email protected]> thanks, -serge _______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
