On Thu, Sep 10, 2009 at 03:42:07PM -0400, Jeff Layton wrote: > A couple of years ago, Bruce committed a patch to make knfsd send > unsigned uid's and gid's to idmapd, rather than signed values. Part > of that earlier discussion is here: > > http://linux-nfs.org/pipermail/nfsv4/2007-December/007321.html > > While this fixed the immediate problem, it doesn't appear that anything > was ever done to make idmapd continue working when it gets a bogus > upcall. Thanks for following up on this! > > idmapd uses libevent for its main event handling loop. When idmapd gets > an upcall from knfsd it will service the request and then rearm the > event by calling event_add on the event structure again. > > When it hits an error though, it returns in most cases w/o rearming the > event. That prevents idmapd from servicing any further requests from > knfsd. > > I've made another change too. If an error is encountered while reading > the channel file, this patch has it close and reopen the file prior to > rearming the event. > > I've not been able to test this patch directly, but I have tested a > backport of it to earlier idmapd code and verified that it did prevent > idmapd from hanging when it got a badly formatted upcall from knfsd. > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > utils/idmapd/idmapd.c | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c > index 65a6a2a..573abaa 100644 > --- a/utils/idmapd/idmapd.c > +++ b/utils/idmapd/idmapd.c > @@ -139,6 +139,7 @@ static void nametoidres(struct idmap_msg *); > > static int nfsdopen(void); > static int nfsdopenone(struct idmap_client *); > +static void nfsdreopen_one(struct idmap_client *); > static void nfsdreopen(void); > > size_t strlcat(char *, const char *, size_t); > @@ -502,7 +503,8 @@ nfsdcb(int fd, short which, void *data) > xlog_warn("nfsdcb: read(%s) failed: errno %d (%s)", > ic->ic_path, len?errno:0, > len?strerror(errno):"End of File"); > - goto out; > + nfsdreopen_one(ic); > + return; Why the "return"? From your description above ("... close and reopen the file prior to rearming the event"), I would have thought you'd want the final event_add() that's done at "out". --b. > } > > /* Get rid of newline and terminate buffer*/ > @@ -514,11 +516,11 @@ nfsdcb(int fd, short which, void *data) > /* Authentication name -- ignored for now*/ > if (getfield(&bp, authbuf, sizeof(authbuf)) == -1) { > xlog_warn("nfsdcb: bad authentication name in upcall\n"); > - return; > + goto out; > } > if (getfield(&bp, typebuf, sizeof(typebuf)) == -1) { > xlog_warn("nfsdcb: bad type in upcall\n"); > - return; > + goto out; > } > if (verbose > 0) > xlog_warn("nfsdcb: authbuf=%s authtype=%s", > @@ -532,26 +534,26 @@ nfsdcb(int fd, short which, void *data) > im.im_conv = IDMAP_CONV_NAMETOID; > if (getfield(&bp, im.im_name, sizeof(im.im_name)) == -1) { > xlog_warn("nfsdcb: bad name in upcall\n"); > - return; > + goto out; > } > break; > case IC_IDNAME: > im.im_conv = IDMAP_CONV_IDTONAME; > if (getfield(&bp, buf1, sizeof(buf1)) == -1) { > xlog_warn("nfsdcb: bad id in upcall\n"); > - return; > + goto out; > } > tmp = strtoul(buf1, (char **)NULL, 10); > im.im_id = (u_int32_t)tmp; > if ((tmp == ULONG_MAX && errno == ERANGE) > || (unsigned long)im.im_id != tmp) { > xlog_warn("nfsdcb: id '%s' too big!\n", buf1); > - return; > + goto out; > } > break; > default: > xlog_warn("nfsdcb: Unknown which type %d", ic->ic_which); > - return; > + goto out; > } > > imconv(ic, &im); > @@ -612,7 +614,7 @@ nfsdcb(int fd, short which, void *data) > break; > default: > xlog_warn("nfsdcb: Unknown which type %d", ic->ic_which); > - return; > + goto out; > } > > bsiz = sizeof(buf) - bsiz; > -- > 1.5.5.6 > > _______________________________________________ > NFSv4 mailing list > NFSv4@xxxxxxxxxxxxx > http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
