idmapd: rearm event handler after error in nfsdcb()

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.

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@redhat.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 65a6a2a2ef413e116357d209f8bbdd2e3cdf7f2d..573abaaead3153817fec3667169d6267c8bf1df2 100644 (file)
--- 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;
        }
 
        /* 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;