[PATCH 1/3] cifs-utils/svcgssd: Fix use-after-free bug (config variables)

[PATCH 1/3] cifs-utils/svcgssd: Fix use-after-free bug (config variables)

[marcel]

From: Marcel Ritter <marcel@linux-ng.de>

This patch fixes a bug when trying to set "principal" in /etc/nfs.conf.
Memory gets freed by conf_cleanup() before being used - moving cleanup
code resolves that.

---
 utils/gssd/svcgssd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index 881207b3..a242b789 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -211,9 +211,6 @@ main(int argc, char *argv[])
 	rpc_verbosity = conf_get_num("svcgssd", "RPC-Verbosity", rpc_verbosity);
 	idmap_verbosity = conf_get_num("svcgssd", "IDMAP-Verbosity", idmap_verbosity);
 
-	/* We don't need the config anymore */
-	conf_cleanup();
-
 	while ((opt = getopt(argc, argv, "fivrnp:")) != -1) {
 		switch (opt) {
 			case 'f':
@@ -328,6 +325,9 @@ main(int argc, char *argv[])
 
 	daemon_ready();
 
+	/* We don't need the config anymore */
+	conf_cleanup();
+
 	nfs4_init_name_mapping(NULL); /* XXX: should only do this once */
 
 	rc = event_base_dispatch(evbase);
-- 
2.34.1

[PATCH 2/3] cifs-utils/svcgssd: Display principal if set

[marcel]

From: Marcel Ritter <marcel@linux-ng.de>

It's a little irritating to only see the template "<...>@<...>" if you
set a specific principal name. So let's show it (if set).

---
 utils/gssd/svcgssd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
index a242b789..ce78d8f7 100644
--- a/utils/gssd/svcgssd.c
+++ b/utils/gssd/svcgssd.c
@@ -295,9 +295,9 @@ main(int argc, char *argv[])
 				(const gss_OID)GSS_C_NT_HOSTBASED_SERVICE);
 		if (status == FALSE) {
 			printerr(0, "unable to obtain root (machine) credentials\n");
-			printerr(0, "do you have a keytab entry for "
-				"nfs/<your.host>@<YOUR.REALM> in "
-				"/etc/krb5.keytab?\n");
+			printerr(0, "do you have a keytab entry for %s in"
+				"/etc/krb5.keytab?\n",
+				principal ? principal : "nfs/<your.host>@<YOUR.REALM>");
 			exit(1);
 		}
 	} else {
-- 
2.34.1

[PATCH 3/3] cifs-utils/svcgssd: Add (undocumented) config options to man page

[marcel]

From: Marcel Ritter <marcel@linux-ng.de>

There seem to be some undocumented options implemented.
Why not mention them in the man page?

---
 utils/gssd/svcgssd.man | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/utils/gssd/svcgssd.man b/utils/gssd/svcgssd.man
index 15ef4c94..8771c035 100644
--- a/utils/gssd/svcgssd.man
+++ b/utils/gssd/svcgssd.man
@@ -61,6 +61,19 @@ this is equivalent to the
 option.  If set to any other value, that is used like the
 .B -p
 option.
+.TP
+.B verbosity
+Value which is equivalent to the number of
+.BR -v .
+.TP
+.B rpc-verbosity
+Value which is equivalent to the number of
+.BR -r .
+.TP
+.B idmap-verbosity
+Value which is equivalent to the number of
+.BR -i .
+
 
 .SH SEE ALSO
 .BR rpc.gssd(8),
-- 
2.34.1

Re: [PATCH 1/3] cifs-utils/svcgssd: Fix use-after-free bug (config variables)

[marcel]

Hi again,

argl - just noticed that I described the patches with "cifs-utils" - should be "nfs-utils" of course :-(
Sorry for that.

Marcel


June 7, 2022 10:19 AM, marcel@linux-ng.de wrote:

> From: Marcel Ritter <marcel@linux-ng.de>
> 
> This patch fixes a bug when trying to set "principal" in /etc/nfs.conf.
> Memory gets freed by conf_cleanup() before being used - moving cleanup
> code resolves that.
> 
> ---
> utils/gssd/svcgssd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
> index 881207b3..a242b789 100644
> --- a/utils/gssd/svcgssd.c
> +++ b/utils/gssd/svcgssd.c
> @@ -211,9 +211,6 @@ main(int argc, char *argv[])
> rpc_verbosity = conf_get_num("svcgssd", "RPC-Verbosity", rpc_verbosity);
> idmap_verbosity = conf_get_num("svcgssd", "IDMAP-Verbosity", idmap_verbosity);
> 
> - /* We don't need the config anymore */
> - conf_cleanup();
> -
> while ((opt = getopt(argc, argv, "fivrnp:")) != -1) {
> switch (opt) {
> case 'f':
> @@ -328,6 +325,9 @@ main(int argc, char *argv[])
> 
> daemon_ready();
> 
> + /* We don't need the config anymore */
> + conf_cleanup();
> +
> nfs4_init_name_mapping(NULL); /* XXX: should only do this once */
> 
> rc = event_base_dispatch(evbase);
> -- 
> 2.34.1

Re: [PATCH 1/3] cifs-utils/svcgssd: Fix use-after-free bug (config variables)

[Steve Dickson]

All 3 patch committed (tag: nfs-utils-2-6-2-rc7)

steved.

On 6/7/22 4:19 AM, marcel@linux-ng.de wrote:
> From: Marcel Ritter <marcel@linux-ng.de>
> 
> This patch fixes a bug when trying to set "principal" in /etc/nfs.conf.
> Memory gets freed by conf_cleanup() before being used - moving cleanup
> code resolves that.
> 
> ---
>   utils/gssd/svcgssd.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
> index 881207b3..a242b789 100644
> --- a/utils/gssd/svcgssd.c
> +++ b/utils/gssd/svcgssd.c
> @@ -211,9 +211,6 @@ main(int argc, char *argv[])
>   	rpc_verbosity = conf_get_num("svcgssd", "RPC-Verbosity", rpc_verbosity);
>   	idmap_verbosity = conf_get_num("svcgssd", "IDMAP-Verbosity", idmap_verbosity);
>   
> -	/* We don't need the config anymore */
> -	conf_cleanup();
> -
>   	while ((opt = getopt(argc, argv, "fivrnp:")) != -1) {
>   		switch (opt) {
>   			case 'f':
> @@ -328,6 +325,9 @@ main(int argc, char *argv[])
>   
>   	daemon_ready();
>   
> +	/* We don't need the config anymore */
> +	conf_cleanup();
> +
>   	nfs4_init_name_mapping(NULL); /* XXX: should only do this once */
>   
>   	rc = event_base_dispatch(evbase);

Re: [PATCH 3/3] cifs-utils/svcgssd: Add (undocumented) config options to man page

[Andreas Hasenack]

On the heels of this patch, you might want to add this tiny bit to
also update the nfs.conf(5) manpage:
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -283,7 +283,10 @@
 .TP
 .B svcgssd
 Recognized values:
-.BR principal .
+.BR principal ,
+.BR verbosity ,
+.BR rpc-verbosity ,
+.BR idmap-verbosity .

 See
 .BR rpc.svcgssd (8)

On Tue, Jun 7, 2022 at 8:19 AM <marcel@linux-ng.de> wrote:
>
> From: Marcel Ritter <marcel@linux-ng.de>
>
> There seem to be some undocumented options implemented.
> Why not mention them in the man page?
>
> ---
>  utils/gssd/svcgssd.man | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/utils/gssd/svcgssd.man b/utils/gssd/svcgssd.man
> index 15ef4c94..8771c035 100644
> --- a/utils/gssd/svcgssd.man
> +++ b/utils/gssd/svcgssd.man
> @@ -61,6 +61,19 @@ this is equivalent to the
>  option.  If set to any other value, that is used like the
>  .B -p
>  option.
> +.TP
> +.B verbosity
> +Value which is equivalent to the number of
> +.BR -v .
> +.TP
> +.B rpc-verbosity
> +Value which is equivalent to the number of
> +.BR -r .
> +.TP
> +.B idmap-verbosity
> +Value which is equivalent to the number of
> +.BR -i .
> +
>
>  .SH SEE ALSO
>  .BR rpc.gssd(8),
> --
> 2.34.1
>