Re: smb daemon get additional command line parameters from env variable

Re: smb daemon get additional command line parameters from env variable

[Laurent Vivier]

Hi,

On 30/10/2019 08:55, Jordi Pujol wrote:
> Hello,
> 
> The new version 4 of Samba disables version 1 SMB protocols, now to
> run old Win clients we must modify the temporary configuration of the
> samba daemon to enable protocols before starting the samba client.
> This patch gets the value of the environment variable SMBDOPTIONS and
> appends it to the smbd command line; it allows the user to specify
> additional samba daemon parameters before starting qemu.
> It also patches the fork_exec function in misc.c, to parse correctly
> command lines that contain spaces.
> 
> Thanks,
> 
> Jordi Pujol
> 

cc'ed: qemu-devel and maintainers

To contribute, please follow the guideline
https://wiki.qemu.org/Contribute/SubmitAPatch

Thanks,
Laurent

Re: [PATCH v2] smb daemon get additional command line parameters from env variable

[Jordi Pujol]

Hello,

Many thanks Laurent,

This is the version 2 of this patch, has been modified according to
the qemu guidelines.

**************************************************
From: Jordi Pujol Palomer <jordipujolp@gmail.com>
Date: Thu, 31 Oct 2019 14:31:14 +0200
Subject: [PATCH v2] QEMU samba daemon: additional command line options

The smbd daemon takes additional command line options
from environment variable SMBDOPTIONS.
Set the environment variable SMBDOPTIONS before executing qemu.

Example:

export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"

Signed-off-by: Jordi Pujol Palomer <jordipujolp@gmail.com>
---
--- qemu-4.1-a/net/slirp.c
+++ qemu_4.1-b/net/slirp.c
@@ -909,6 +909,12 @@ static int slirp_smb(SlirpState* s, cons
              CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
     g_free(smb_conf);

+    char *options = g_strdup(g_getenv("SMBDOPTIONS"));
+    if (options) {
+        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
+    }
+    g_free(options);
+
     if (slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 139) < 0 ||
         slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 445) < 0) {
         slirp_smb_cleanup(s);

--- qemu-4.1-a/slirp/src/misc.c 2019-10-29 14:40:15.043120941 +0100
+++ qemu-4.1-b/slirp/src/misc.c 2019-10-29 14:41:04.440235684 +0100
@@ -168,7 +168,9 @@ g_spawn_async_with_fds_slirp(const gchar
 int fork_exec(struct socket *so, const char *ex)
 {
     GError *err = NULL;
-    char **argv;
+    gint argc = 0;
+    gchar **argv = NULL;
+    gboolean ret;
     int opt, sp[2];

     DEBUG_CALL("fork_exec");
@@ -179,7 +181,13 @@ int fork_exec(struct socket *so, const c
         return 0;
     }

-    argv = g_strsplit(ex, " ", -1);
+    ret = g_shell_parse_argv(ex, &argc, &argv, &err);
+    if (err) {
+        g_critical("fork_exec invalid command: %s", err->message);
+        g_error_free(err);
+        return 0;
+    }
+
     g_spawn_async_with_fds(NULL /* cwd */, argv, NULL /* env */,
                            G_SPAWN_SEARCH_PATH, fork_exec_child_setup,
                            NULL /* data */, NULL /* child_pid */, sp[1], sp[1],
**************************************************

Thanks,

Jordi Pujol

Re: [PATCH v2] smb daemon get additional command line parameters from env variable

[Samuel Thibault]

Hello,

Jordi Pujol, le jeu. 31 oct. 2019 14:33:00 +0100, a ecrit:
> The smbd daemon takes additional command line options
> from environment variable SMBDOPTIONS.
> Set the environment variable SMBDOPTIONS before executing qemu.
> 
> Example:
> 
> export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"
> 
> Signed-off-by: Jordi Pujol Palomer <jordipujolp@gmail.com>

> ---
> --- qemu-4.1-a/net/slirp.c
> +++ qemu_4.1-b/net/slirp.c
> @@ -909,6 +909,12 @@ static int slirp_smb(SlirpState* s, cons
>               CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
>      g_free(smb_conf);
> 
> +    char *options = g_strdup(g_getenv("SMBDOPTIONS"));

Why strduping it? you can just use g_getenv.

> +    if (options) {
> +        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +    }
> +    g_free(options);
> +
>      if (slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 139) < 0 ||
>          slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 445) < 0) {
>          slirp_smb_cleanup(s);
> 

> --- qemu-4.1-a/slirp/src/misc.c 2019-10-29 14:40:15.043120941 +0100
> +++ qemu-4.1-b/slirp/src/misc.c 2019-10-29 14:41:04.440235684 +0100

Please submit this part to https://gitlab.freedesktop.org/slirp/libslirp/

Make sure to note in the changelog that g_shell_parse_argv does only
tokenization, and no replacement.

Samuel

> @@ -168,7 +168,9 @@ g_spawn_async_with_fds_slirp(const gchar
>  int fork_exec(struct socket *so, const char *ex)
>  {
>      GError *err = NULL;
> -    char **argv;
> +    gint argc = 0;
> +    gchar **argv = NULL;
> +    gboolean ret;
>      int opt, sp[2];
> 
>      DEBUG_CALL("fork_exec");
> @@ -179,7 +181,13 @@ int fork_exec(struct socket *so, const c
>          return 0;
>      }
> 
> -    argv = g_strsplit(ex, " ", -1);
> +    ret = g_shell_parse_argv(ex, &argc, &argv, &err);
> +    if (err) {
> +        g_critical("fork_exec invalid command: %s", err->message);
> +        g_error_free(err);
> +        return 0;
> +    }
> +
>      g_spawn_async_with_fds(NULL /* cwd */, argv, NULL /* env */,
>                             G_SPAWN_SEARCH_PATH, fork_exec_child_setup,
>                             NULL /* data */, NULL /* child_pid */, sp[1], sp[1],
> **************************************************
> 
> Thanks,
> 
> Jordi Pujol
> 

Re: [PATCH v3] smb daemon get additional command line parameters from env variable

[Jordi Pujol]

On Thu, Oct 31, 2019 at 11:50 PM Samuel Thibault
<samuel.thibault@gnu.org> wrote:
> Why strduping it? you can just use g_getenv.

ACK, I have also placed the variable declaration after the others.
Here is the v3 of this patch.

******************************************************************
From: Jordi Pujol Palomer <jordipujolp@gmail.com>
Date: Fri, 1 Nov 2019 10:54:14 +0200
Subject: [PATCH v3] QEMU samba daemon: additional command line options

The smbd daemon takes additional command line options
from environment variable SMBDOPTIONS.
Set the environment variable SMBDOPTIONS before executing qemu.

Example:

export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"

Signed-off-by: Jordi Pujol Palomer <jordipujolp@gmail.com>
---
--- qemu-4.1-a/net/slirp.c
+++ qemu_4.1-b/net/slirp.c
@@ -834,6 +834,7 @@ static int slirp_smb(SlirpState* s, cons
     char *smb_cmdline;
     struct passwd *passwd;
     FILE *f;
+    char *options;

     passwd = getpwuid(geteuid());
     if (!passwd) {
@@ -909,6 +910,12 @@ static int slirp_smb(SlirpState* s, cons
              CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
     g_free(smb_conf);

+    options = g_getenv("SMBDOPTIONS");
+    if (options) {
+        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
+    }
+    g_free(options);
+
     if (slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 139) < 0 ||
         slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 445) < 0) {
         slirp_smb_cleanup(s);
******************************************************************

>
> Please submit this part to https://gitlab.freedesktop.org/slirp/libslirp/

I have forked the libslirp project and created a merge request,

Thanks,

Jordi Pujol

Re: [PATCH v3] smb daemon get additional command line parameters from env variable

[Samuel Thibault]

Jordi Pujol, le ven. 01 nov. 2019 15:38:19 +0100, a ecrit:
> +    options = g_getenv("SMBDOPTIONS");
> +    if (options) {
> +        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +    }
> +    g_free(options);

But then do not free it :)

Samuel

Re: [PATCH v4] smb daemon get additional command line parameters from env variable

[Jordi Pujol]

From: Jordi Pujol Palomer <jordipujolp@gmail.com>
Date: Sat, 2 Nov 2019 08:54:14 +0200
Subject: [PATCH v4] QEMU samba daemon: additional command line options

The smbd daemon takes additional command line options
from environment variable SMBDOPTIONS.
Set the environment variable SMBDOPTIONS before executing qemu.

Example:

export SMBDOPTIONS="--option='server min protocol=CORE' -d 4"

Signed-off-by: Jordi Pujol Palomer <jordipujolp@gmail.com>
---
--- qemu-4.1-a/net/slirp.c
+++ qemu_4.1-b/net/slirp.c
@@ -834,6 +834,7 @@ static int slirp_smb(SlirpState* s, cons
     char *smb_cmdline;
     struct passwd *passwd;
     FILE *f;
+    char *options;

     passwd = getpwuid(geteuid());
     if (!passwd) {
@@ -909,6 +910,12 @@ static int slirp_smb(SlirpState* s, cons
              CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
     g_free(smb_conf);

+    options = g_getenv("SMBDOPTIONS");
+    if (options) {
+        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
+        g_free(options);
+    }
+
     if (slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 139) < 0 ||
         slirp_add_exec(s->slirp, smb_cmdline, &vserver_addr, 445) < 0) {
         slirp_smb_cleanup(s);

Re: [PATCH v4] smb daemon get additional command line parameters from env variable

[Samuel Thibault]

Jordi Pujol, le sam. 02 nov. 2019 08:41:52 +0100, a ecrit:
> @@ -909,6 +910,12 @@ static int slirp_smb(SlirpState* s, cons
>               CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);
>      g_free(smb_conf);
> 
> +    options = g_getenv("SMBDOPTIONS");
> +    if (options) {
> +        smb_cmdline = g_strdup_printf("%s %s", smb_cmdline, options);
> +        g_free(options);
> +    }

Again, what g_getenv mustn't be freed. I believe you even get a warning
about it: g_getenv returns a const gchar *.

The old value of smb_cmdline, however, has to be freed, otherwise that's
a memory leak.

Samuel