On Wed, Jul 24, 2019 at 04:28:03PM +0800, Jia-Ju Bai wrote: > In nfs4_xdr_dec_cb_recall(), nfs4_xdr_dec_cb_layout() and > nfs4_xdr_dec_cb_notify_lock(), there is an if statement to check whether > cb is NULL. > > When cb is NULL, the three functions all call: > decode_cb_op_status(..., &cb->cb_status); > > Thus, possible null-pointer dereferences may occur. > > To fix these possible bugs, -EINVAL is returned when cb is NULL. > > These bugs are found by a static analysis tool STCheck written by us.
Thanks! But I think actually the correct fix is just to remove the NULL checks entirely. --b. > > Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com> > --- > fs/nfsd/nfs4callback.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index 397eb7820929..55949a158b6b 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -516,7 +516,8 @@ static int nfs4_xdr_dec_cb_recall(struct rpc_rqst *rqstp, > status = decode_cb_sequence4res(xdr, cb); > if (unlikely(status || cb->cb_seq_status)) > return status; > - } > + } else > + return -EINVAL; > > return decode_cb_op_status(xdr, OP_CB_RECALL, &cb->cb_status); > } > @@ -608,7 +609,9 @@ static int nfs4_xdr_dec_cb_layout(struct rpc_rqst *rqstp, > status = decode_cb_sequence4res(xdr, cb); > if (unlikely(status || cb->cb_seq_status)) > return status; > - } > + } else > + return -EINVAL; > + > return decode_cb_op_status(xdr, OP_CB_LAYOUTRECALL, &cb->cb_status); > } > #endif /* CONFIG_NFSD_PNFS */ > @@ -667,7 +670,9 @@ static int nfs4_xdr_dec_cb_notify_lock(struct rpc_rqst > *rqstp, > status = decode_cb_sequence4res(xdr, cb); > if (unlikely(status || cb->cb_seq_status)) > return status; > - } > + } else > + return -EINVAL; > + > return decode_cb_op_status(xdr, OP_CB_NOTIFY_LOCK, &cb->cb_status); > } > > -- > 2.17.0
