| Message ID | da06a514114528e639923b8ef26c0e815e91554f.1448031050.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ben, On Fri, Nov 20, 2015 at 9:56 AM, Benjamin Coddington <bcodding@redhat.com> wrote: > > If clp->cl_cb_ident is zero, then nfs_cb_idr_remove_locked() skips removing > it when the nfs_client is freed. A decoding or server bug can then find > and try to put that first nfs_client which would lead to a crash. Thanks for fixing! Have you ever seen such a crash in the wild? IOW: is this something we should consider for stable? Cheers Trond -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 20 Nov 2015, Trond Myklebust wrote: > Hi Ben, > > On Fri, Nov 20, 2015 at 9:56 AM, Benjamin Coddington > <bcodding@redhat.com> wrote: > > > > If clp->cl_cb_ident is zero, then nfs_cb_idr_remove_locked() skips removing > > it when the nfs_client is freed. A decoding or server bug can then find > > and try to put that first nfs_client which would lead to a crash. > > Thanks for fixing! > Have you ever seen such a crash in the wild? IOW: is this something we > should consider for stable? Yes, there's a server that sometimes sends truncated cb_compounds, and this bug is hit if we continue decoding 0's past the end of received data since cb_ident ends up being 0. Both fixes I sent today are trying to handle that particular behavior. Ben -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 20, 2015 at 11:56 AM, Benjamin Coddington <bcodding@redhat.com> wrote: > On Fri, 20 Nov 2015, Trond Myklebust wrote: > >> Hi Ben, >> >> On Fri, Nov 20, 2015 at 9:56 AM, Benjamin Coddington >> <bcodding@redhat.com> wrote: >> > >> > If clp->cl_cb_ident is zero, then nfs_cb_idr_remove_locked() skips removing >> > it when the nfs_client is freed. A decoding or server bug can then find >> > and try to put that first nfs_client which would lead to a crash. >> >> Thanks for fixing! >> Have you ever seen such a crash in the wild? IOW: is this something we >> should consider for stable? > > Yes, there's a server that sometimes sends truncated cb_compounds, and this > bug is hit if we continue decoding 0's past the end of received data since > cb_ident ends up being 0. Both fixes I sent today are trying to handle that > particular behavior. > OK. I'll mark both for stable inclusion then. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c index 223bedd..10410e8 100644 --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -33,7 +33,7 @@ static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion) return ret; idr_preload(GFP_KERNEL); spin_lock(&nn->nfs_client_lock); - ret = idr_alloc(&nn->cb_ident_idr, clp, 0, 0, GFP_NOWAIT); + ret = idr_alloc(&nn->cb_ident_idr, clp, 1, 0, GFP_NOWAIT); if (ret >= 0) clp->cl_cb_ident = ret; spin_unlock(&nn->nfs_client_lock);
If clp->cl_cb_ident is zero, then nfs_cb_idr_remove_locked() skips removing it when the nfs_client is freed. A decoding or server bug can then find and try to put that first nfs_client which would lead to a crash. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/nfs4client.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)