[PATCH 0/2] unshare: some idmap fixes

[PATCH 0/2] unshare: some idmap fixes

[Sean Anderson]

This series has some fixes to code and documentation around idmaps.


Sean Anderson (2):
  unshare: Fix parsing of id maps
  unshare: Fix doc comments

 sys-utils/unshare.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/2] unshare: Fix parsing of id maps

[Sean Anderson]

For whatever reason, mem2strcpy places the nul-terminator at the end of
the buffer instead of at the end of the string it copies. This makes it
completely useless for our purposes, since one would have to add a
terminator manually to avoid getting garbage. Just use memcpy instead.

Fixes: ff5dc96eb ("unshare: Add options to map blocks of user/group IDs")
Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reported-by: Daniel Gerber <dg@atufi.org>
---

 sys-utils/unshare.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 443358952..889c561ca 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -387,8 +387,9 @@ static int uint_to_id(const char *name, size_t sz)
 {
 	char buf[UID_BUFSIZ];
 
-	mem2strcpy(buf, name, sz, sizeof(buf));
-	return strtoul_or_err(name, _("could not parse ID"));
+	memcpy(buf, name, min(sz, sizeof(buf) - 1));
+	buf[sz] = '\0';
+	return strtoul_or_err(buf, _("could not parse ID"));
 }
 
 /**
-- 
2.34.1

[PATCH 2/2] unshare: Fix doc comments

[Sean Anderson]

Several doc comments use the wrong terminology, or have mismatches
descriptions. Fix them.

Fixes: ff5dc96eb ("unshare: Add options to map blocks of user/group IDs")
Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reported-by: Daniel Gerber <dg@atufi.org>
---

 sys-utils/unshare.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 889c561ca..edce65c77 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -359,8 +359,8 @@ static gid_t get_group(const char *s, const char *err)
 
 /**
  * struct map_range - A range of IDs to map
- * @outer: First ID inside the namespace
- * @inner: First ID outside the namespace
+ * @outer: First ID mapped on the outside of the namespace
+ * @inner: First ID mapped on the inside of the namespace
  * @count: Length of the inside and outside ranges
  *
  * A range of uids/gids to map using new[gu]idmap.
@@ -394,9 +394,9 @@ static int uint_to_id(const char *name, size_t sz)
 
 /**
  * get_map_range() - Parse a mapping range from a string
- * @s: A string of the format upper,lower,count
+ * @s: A string of the format outer,inner,count
  *
- * Parse a string of the form upper,lower,count into a new mapping range.
+ * Parse a string of the form outer,inner,count into a new mapping range.
  *
  * Return: A new &struct map_range
  */
-- 
2.34.1

Re: [PATCH 1/2] unshare: Fix parsing of id maps

[Karel Zak]

On Sat, Jan 15, 2022 at 11:29:25AM -0500, Sean Anderson wrote:
> For whatever reason, mem2strcpy places the nul-terminator at the end of
> the buffer

Yes, it was originally designed for utmp-like strings where rest of
the buffer is filled by nul-terminators. I think we can fix it to make
it usable for normal strings too.

> instead of at the end of the string it copies. This makes it
> completely useless for our purposes, since one would have to add a
> terminator manually to avoid getting garbage. Just use memcpy instead.
> 
> Fixes: ff5dc96eb ("unshare: Add options to map blocks of user/group IDs")
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reported-by: Daniel Gerber <dg@atufi.org>
> ---
> 
>  sys-utils/unshare.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
> index 443358952..889c561ca 100644
> --- a/sys-utils/unshare.c
> +++ b/sys-utils/unshare.c
> @@ -387,8 +387,9 @@ static int uint_to_id(const char *name, size_t sz)
>  {
>  	char buf[UID_BUFSIZ];
>  
> -	mem2strcpy(buf, name, sz, sizeof(buf));
> -	return strtoul_or_err(name, _("could not parse ID"));
> +	memcpy(buf, name, min(sz, sizeof(buf) - 1));
> +	buf[sz] = '\0';
        ^^
What about sz > sizeof(buf)?

Maybe it would be enough to improve mem2strcpy() in include/strutils.h 

 - dest[nmax-1] = '\0';
 + dest[n] = '\0';

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] unshare: some idmap fixes

[Karel Zak]

On Sat, Jan 15, 2022 at 11:29:24AM -0500, Sean Anderson wrote:
> This series has some fixes to code and documentation around idmaps.
> 
> 
> Sean Anderson (2):
>   unshare: Fix parsing of id maps

I have fixed mem2strcpy().

>   unshare: Fix doc comments

Applied. Thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com