diff mbox

mount: call setgroups() before setuid()

Message ID 877f51wqye.fsf@notabene.neil.brown.name (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown Feb. 7, 2017, 10:56 p.m. UTC
It is generally wise to call setgroups() (and setgid()) before calling
setuid() to ensure no unexpected permission leaks happen.
SUSE's build system checks all binaries for conformance with this
and generates a warning for mountd.

As we are setting the uid to 0, there is no risk that the group list
will provide extra permissions, so there is no real risk here.
But it is nice to silence warnings, and including a setgroups()
call is probably a good practice to encourage.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 utils/mount/network.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Steve Dickson Feb. 16, 2017, 10:22 a.m. UTC | #1
On 02/07/2017 05:56 PM, NeilBrown wrote:
> It is generally wise to call setgroups() (and setgid()) before calling
> setuid() to ensure no unexpected permission leaks happen.
> SUSE's build system checks all binaries for conformance with this
> and generates a warning for mountd.
>
> As we are setting the uid to 0, there is no risk that the group list
> will provide extra permissions, so there is no real risk here.
> But it is nice to silence warnings, and including a setgroups()
> call is probably a good practice to encourage.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Committed...

steved.

> ---
>  utils/mount/network.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index d1c8fec75174..281e9354a7fa 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -33,6 +33,7 @@
>  #include <errno.h>
>  #include <netdb.h>
>  #include <time.h>
> +#include <grp.h>
>  
>  #include <sys/types.h>
>  #include <sys/socket.h>
> @@ -804,6 +805,7 @@ int start_statd(void)
>  			pid_t pid = fork();
>  			switch (pid) {
>  			case 0: /* child */
> +				setgroups(0, NULL);
>  				setgid(0);
>  				setuid(0);
>  				execle(START_STATD, START_STATD, NULL, envp);

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils/mount/network.c b/utils/mount/network.c
index d1c8fec75174..281e9354a7fa 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -33,6 +33,7 @@ 
 #include <errno.h>
 #include <netdb.h>
 #include <time.h>
+#include <grp.h>
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -804,6 +805,7 @@  int start_statd(void)
 			pid_t pid = fork();
 			switch (pid) {
 			case 0: /* child */
+				setgroups(0, NULL);
 				setgid(0);
 				setuid(0);
 				execle(START_STATD, START_STATD, NULL, envp);