--- a/sys/vfs/nfs/nfs_vfsops.c +++ b/sys/vfs/nfs/nfs_vfsops.c @@ -1225,10 +1225,20 @@ /* * We are now committed to the unmount. - * For NQNFS, let the server daemon free the nfsmount structure. + * + * If an nfs_clientd() is (or may be) looping against this mount we + * must NOT free the nfsmount here: nfs_clientd() keeps running + * against nmp after VFS_UNMOUNT returns (its loop only notices the + * dismount at its next wakeup, up to hz/3 later). Flag the + * dismount as the last thing we do to nmp and delegate the free to + * nfs_clientd(). + * + * This covers both kerberos mounts (the historical case) and any + * mount where nfssvc(NFSSVC_MNTD) entered nfs_clientd() (the kernel + * does not restrict MNTD to kerberos mounts), fixing a + * use-after-free / double free of the nfsmount. It also stops + * leaking the nfsmount of kerb mounts whose clientd never ran. */ - if (nmp->nm_flag & NFSMNT_KERB) - nmp->nm_state |= NFSSTA_DISMNT; nfssvc_iod_stop1(nmp); nfs_disconnect(nmp); nfssvc_iod_stop2(nmp); @@ -1237,9 +1247,18 @@ TAILQ_REMOVE(&nfs_mountq, nmp, nm_entry); lwkt_reltoken(&nfs_token); + if (nmp->nm_state & NFSSTA_MNTD) { + /* + * nfs_clientd() is active: it polls nm_state at least + * every hz/3 and frees nmp once it sees NFSSTA_DISMNT. + * Set the flag only after our own last use of nmp above. + */ + nmp->nm_state |= NFSSTA_DISMNT; + wakeup((caddr_t)&nmp->nm_authstr); + } lwkt_reltoken(&nmp->nm_token); - if ((nmp->nm_flag & NFSMNT_KERB) == 0) { + if ((nmp->nm_state & NFSSTA_MNTD) == 0) { nfs_free_mount(nmp); } return (0); --- a/sys/vfs/nfs/nfs_kerb.c +++ b/sys/vfs/nfs/nfs_kerb.c @@ -122,8 +122,11 @@ (nmp->nm_state & (NFSSTA_WAITAUTH | NFSSTA_HASAUTH))) { error = tsleep((caddr_t)&nmp->nm_authstr, PCATCH, "nqnfstimr", hz / 3); - if (error == EINTR || error == ERESTART) - (void) dounmount(nmp->nm_mountp, 0, 0); + if (error == EINTR || error == ERESTART) { + if (nmp->nm_state & NFSSTA_DISMNT) + break; /* unmount done, mp already freed */ + (void) dounmount(nmp->nm_mountp, 0, 0); + } } }