* [PATCH 0/2] Implement fgetspent @ 2013-11-21 3:13 Michael Forney 2013-11-21 3:13 ` [PATCH 1/2] shadow: Move spent parsing to internal function Michael Forney ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Michael Forney @ 2013-11-21 3:13 UTC (permalink / raw) To: musl Hi Rich, It turns out that one more function, fgetspent, is needed for the common shadow+pam_unix setup used in many desktop systems. Otherwise, when changing your password, pam_unix will attempt to enumerate through the shadow entries with fgetspent, rewriting them as it goes (and replacing the entry to be updated). This leaves you with a shadow file containing only the updated entry. In the first patch, I moved spent parsing to an internal function, __parsespent. I opted to use __parsespent instead of __getspent_a (similar to the passwd and group functions) for several reasons: - To minimize the changes necessary to getspnam_r - To avoid the extra memcpy as in getpw_r - It seemed like a more self-contained function (which didn't rely on the source of the entry). - It would make it easier to implement sgetspent if we ever wanted that (though, so far, I haven't found anything that requires this) However, if this is not desired, I can send a new patch which uses a function __getspent_a, similar to __get{pw,gr}ent_a. Michael Forney (2): shadow: Move spent parsing to internal function shadow: Implement fgetspent src/passwd/fgetspent.c | 11 ++++++++++- src/passwd/getspnam_r.c | 34 +--------------------------------- src/passwd/parsespent.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/passwd/pwf.h | 2 +- 4 files changed, 54 insertions(+), 35 deletions(-) create mode 100644 src/passwd/parsespent.c -- 1.8.4.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] shadow: Move spent parsing to internal function 2013-11-21 3:13 [PATCH 0/2] Implement fgetspent Michael Forney @ 2013-11-21 3:13 ` Michael Forney 2013-11-24 6:17 ` [PATCH v2 " Michael Forney 2013-11-21 3:13 ` [PATCH 2/2] shadow: Implement fgetspent Michael Forney 2013-11-23 21:08 ` [PATCH 0/2] " Rich Felker 2 siblings, 1 reply; 5+ messages in thread From: Michael Forney @ 2013-11-21 3:13 UTC (permalink / raw) To: musl --- src/passwd/getspnam_r.c | 34 +--------------------------------- src/passwd/parsespent.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/passwd/pwf.h | 2 +- 3 files changed, 44 insertions(+), 34 deletions(-) create mode 100644 src/passwd/parsespent.c diff --git a/src/passwd/getspnam_r.c b/src/passwd/getspnam_r.c index f4d7b35..59a84b3 100644 --- a/src/passwd/getspnam_r.c +++ b/src/passwd/getspnam_r.c @@ -12,11 +12,6 @@ * file. It also avoids any allocation to prevent memory-exhaustion * attacks via huge TCB shadow files. */ -static long xatol(const char *s) -{ - return isdigit(*s) ? atol(s) : -1; -} - static void cleanup(void *p) { fclose(p); @@ -29,7 +24,6 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct int rv = 0; int fd; size_t k, l = strlen(name); - char *s; int skip = 0; int cs; @@ -71,34 +65,8 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct rv = ERANGE; break; } - buf[k-1] = 0; - - s = buf; - sp->sp_namp = s; - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_pwdp = s; - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_lstchg = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_min = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_max = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_warn = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_inact = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_expire = xatol(s); - if (!(s = strchr(s, ':'))) continue; - *s++ = 0; sp->sp_flag = xatol(s); + if (__parsespent(buf, sp) < 0) continue; *res = sp; break; } diff --git a/src/passwd/parsespent.c b/src/passwd/parsespent.c new file mode 100644 index 0000000..d781387 --- /dev/null +++ b/src/passwd/parsespent.c @@ -0,0 +1,42 @@ +#include "pwf.h" + +static long xatol(char **s) +{ + long x; + if (**s == ':' || **s == '\n') return -1; + for (x=0; **s-'0'<10U; ++*s) x=10*x+(**s-'0'); + return x; +} + +int __parsespent(char *s, struct spwd *sp) +{ + sp->sp_namp = s; + if (!(s = strchr(s, ':'))) return -1; + *s = 0; + + sp->sp_pwdp = ++s; + if (!(s = strchr(s, ':'))) return -1; + *s = 0; + + s++; sp->sp_lstchg = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_min = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_max = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_warn = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_inact = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_expire = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_flag = xatol(&s); + if (*s != '\n') return -1; + return 0; +} diff --git a/src/passwd/pwf.h b/src/passwd/pwf.h index 0a76ef8..2d813ad 100644 --- a/src/passwd/pwf.h +++ b/src/passwd/pwf.h @@ -9,5 +9,5 @@ #include "libc.h" struct passwd *__getpwent_a(FILE *f, struct passwd *pw, char **line, size_t *size); -struct spwd *__getspent_a(FILE *f, struct spwd *sp, char **line, size_t *size); struct group *__getgrent_a(FILE *f, struct group *gr, char **line, size_t *size, char ***mem, size_t *nmem); +int __parsespent(char *s, struct spwd *sp); -- 1.8.4.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] shadow: Move spent parsing to internal function 2013-11-21 3:13 ` [PATCH 1/2] shadow: Move spent parsing to internal function Michael Forney @ 2013-11-24 6:17 ` Michael Forney 0 siblings, 0 replies; 5+ messages in thread From: Michael Forney @ 2013-11-24 6:17 UTC (permalink / raw) To: musl --- src/passwd/getspnam_r.c | 69 ++++++++++++++++++++++++++++--------------------- src/passwd/pwf.h | 2 +- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/passwd/getspnam_r.c b/src/passwd/getspnam_r.c index f4d7b35..15f8c87 100644 --- a/src/passwd/getspnam_r.c +++ b/src/passwd/getspnam_r.c @@ -12,9 +12,45 @@ * file. It also avoids any allocation to prevent memory-exhaustion * attacks via huge TCB shadow files. */ -static long xatol(const char *s) +static long xatol(char **s) { - return isdigit(*s) ? atol(s) : -1; + long x; + if (**s == ':' || **s == '\n') return -1; + for (x=0; **s-'0'<10U; ++*s) x=10*x+(**s-'0'); + return x; +} + +int __parsespent(char *s, struct spwd *sp) +{ + sp->sp_namp = s; + if (!(s = strchr(s, ':'))) return -1; + *s = 0; + + sp->sp_pwdp = ++s; + if (!(s = strchr(s, ':'))) return -1; + *s = 0; + + s++; sp->sp_lstchg = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_min = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_max = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_warn = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_inact = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_expire = xatol(&s); + if (*s != ':') return -1; + + s++; sp->sp_flag = xatol(&s); + if (*s != '\n') return -1; + return 0; } static void cleanup(void *p) @@ -29,7 +65,6 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct int rv = 0; int fd; size_t k, l = strlen(name); - char *s; int skip = 0; int cs; @@ -71,34 +106,8 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct rv = ERANGE; break; } - buf[k-1] = 0; - - s = buf; - sp->sp_namp = s; - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_pwdp = s; - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_lstchg = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_min = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_max = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_warn = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_inact = xatol(s); - if (!(s = strchr(s, ':'))) continue; - - *s++ = 0; sp->sp_expire = xatol(s); - if (!(s = strchr(s, ':'))) continue; - *s++ = 0; sp->sp_flag = xatol(s); + if (__parsespent(buf, sp) < 0) continue; *res = sp; break; } diff --git a/src/passwd/pwf.h b/src/passwd/pwf.h index 0a76ef8..2d813ad 100644 --- a/src/passwd/pwf.h +++ b/src/passwd/pwf.h @@ -9,5 +9,5 @@ #include "libc.h" struct passwd *__getpwent_a(FILE *f, struct passwd *pw, char **line, size_t *size); -struct spwd *__getspent_a(FILE *f, struct spwd *sp, char **line, size_t *size); struct group *__getgrent_a(FILE *f, struct group *gr, char **line, size_t *size, char ***mem, size_t *nmem); +int __parsespent(char *s, struct spwd *sp); -- 1.8.4.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] shadow: Implement fgetspent 2013-11-21 3:13 [PATCH 0/2] Implement fgetspent Michael Forney 2013-11-21 3:13 ` [PATCH 1/2] shadow: Move spent parsing to internal function Michael Forney @ 2013-11-21 3:13 ` Michael Forney 2013-11-23 21:08 ` [PATCH 0/2] " Rich Felker 2 siblings, 0 replies; 5+ messages in thread From: Michael Forney @ 2013-11-21 3:13 UTC (permalink / raw) To: musl --- src/passwd/fgetspent.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/passwd/fgetspent.c b/src/passwd/fgetspent.c index 3dda784..47473bd 100644 --- a/src/passwd/fgetspent.c +++ b/src/passwd/fgetspent.c @@ -1,6 +1,15 @@ #include "pwf.h" +#include <pthread.h> struct spwd *fgetspent(FILE *f) { - return 0; + static char *line; + static struct spwd sp; + size_t size = 0; + struct spwd *res = 0; + int cs; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); + if (getline(&line, &size, f) >= 0 && __parsespent(line, &sp) >= 0) res = &sp; + pthread_setcancelstate(cs, 0); + return res; } -- 1.8.4.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Implement fgetspent 2013-11-21 3:13 [PATCH 0/2] Implement fgetspent Michael Forney 2013-11-21 3:13 ` [PATCH 1/2] shadow: Move spent parsing to internal function Michael Forney 2013-11-21 3:13 ` [PATCH 2/2] shadow: Implement fgetspent Michael Forney @ 2013-11-23 21:08 ` Rich Felker 2 siblings, 0 replies; 5+ messages in thread From: Rich Felker @ 2013-11-23 21:08 UTC (permalink / raw) To: musl On Wed, Nov 20, 2013 at 07:13:39PM -0800, Michael Forney wrote: > Hi Rich, > > It turns out that one more function, fgetspent, is needed for the common > shadow+pam_unix setup used in many desktop systems. Otherwise, when > changing your password, pam_unix will attempt to enumerate through the > shadow entries with fgetspent, rewriting them as it goes (and replacing > the entry to be updated). This leaves you with a shadow file containing > only the updated entry. > > In the first patch, I moved spent parsing to an internal function, > __parsespent. I opted to use __parsespent instead of __getspent_a > (similar to the passwd and group functions) for several reasons: > > - To minimize the changes necessary to getspnam_r > - To avoid the extra memcpy as in getpw_r > - It seemed like a more self-contained function (which didn't rely > on the source of the entry). > - It would make it easier to implement sgetspent if we ever wanted > that (though, so far, I haven't found anything that requires this) > > However, if this is not desired, I can send a new patch which uses a > function __getspent_a, similar to __get{pw,gr}ent_a. Overall I like this, but I don't see any advantage to putting __parsespent in a separate file. On the surface it appears to reduce dependencies for static linking, but any real-world use of fgetspent would already depend on most of the functions getspnam_r depends on. On the other hand, each additional file contributes to build time, static library size (from elf header overhead), and potentially hitting the command line limits on the ar/ld command lines. If you don't have any objections, how about leaving the parsing code in getspnam_r.c and only adding a new file for fgetspent? Rich ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-24 6:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-21 3:13 [PATCH 0/2] Implement fgetspent Michael Forney 2013-11-21 3:13 ` [PATCH 1/2] shadow: Move spent parsing to internal function Michael Forney 2013-11-24 6:17 ` [PATCH v2 " Michael Forney 2013-11-21 3:13 ` [PATCH 2/2] shadow: Implement fgetspent Michael Forney 2013-11-23 21:08 ` [PATCH 0/2] " Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).