| Message ID | 167909365790.1672.13118429954916842449.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | rq_pages bounds checking | expand |
On Fri, 2023-03-17 at 19:01 -0400, Chuck Lever wrote: > A slightly modified take on Jeff's earlier patches, tested with > both NFSv3 and NFSv4.1 via simple fault injection in > svc_rqst_replace_page(). > > In general I'm in favor of more rq_pages bounds checking by > replacing direct modification of the rq_respages and rq_next_page > fields with accessor functions. > > --- > > Chuck Lever (2): > SUNRPC: add bounds checking to svc_rqst_replace_page > NFSD: Watch for rq_pages bounds checking errors in nfsd_splice_actor() > > Jeff Layton (1): > nfsd: don't replace page in rq_pages if it's a continuation of last page > > > fs/nfsd/vfs.c | 15 +++++++++++++-- > include/linux/sunrpc/svc.h | 2 +- > include/trace/events/sunrpc.h | 25 +++++++++++++++++++++++++ > net/sunrpc/svc.c | 15 ++++++++++++++- > 4 files changed, 53 insertions(+), 4 deletions(-) > > -- > Chuck Lever > Looks good, Chuck, thanks. You can add this to the last two: Reviewed-by: Jeff Layton <jlayton@kernel.org>
> On Mar 18, 2023, at 6:04 AM, Jeff Layton <jlayton@kernel.org> wrote: > > On Fri, 2023-03-17 at 19:01 -0400, Chuck Lever wrote: >> A slightly modified take on Jeff's earlier patches, tested with >> both NFSv3 and NFSv4.1 via simple fault injection in >> svc_rqst_replace_page(). >> >> In general I'm in favor of more rq_pages bounds checking by >> replacing direct modification of the rq_respages and rq_next_page >> fields with accessor functions. >> >> --- >> >> Chuck Lever (2): >> SUNRPC: add bounds checking to svc_rqst_replace_page >> NFSD: Watch for rq_pages bounds checking errors in nfsd_splice_actor() >> >> Jeff Layton (1): >> nfsd: don't replace page in rq_pages if it's a continuation of last page >> >> >> fs/nfsd/vfs.c | 15 +++++++++++++-- >> include/linux/sunrpc/svc.h | 2 +- >> include/trace/events/sunrpc.h | 25 +++++++++++++++++++++++++ >> net/sunrpc/svc.c | 15 ++++++++++++++- >> 4 files changed, 53 insertions(+), 4 deletions(-) >> >> -- >> Chuck Lever >> > > Looks good, Chuck, thanks. You can add this to the last two: > > Reviewed-by: Jeff Layton <jlayton@kernel.org> Excellent, thanks! When I started I expected 3/3 to be more substantial, but since it's just a handful of lines and the patch descriptions are about the same, I'm going to squash 2/3 and 3/3 together. Only question is whether to apply that to nfsd-next or nfsd-fixes. Since it's a defensive change, I was thinking nfsd-next. Let me know if you think it should get merged sooner. -- Chuck Lever
On Sat, 2023-03-18 at 15:20 +0000, Chuck Lever III wrote: > > > On Mar 18, 2023, at 6:04 AM, Jeff Layton <jlayton@kernel.org> wrote: > > > > On Fri, 2023-03-17 at 19:01 -0400, Chuck Lever wrote: > > > A slightly modified take on Jeff's earlier patches, tested with > > > both NFSv3 and NFSv4.1 via simple fault injection in > > > svc_rqst_replace_page(). > > > > > > In general I'm in favor of more rq_pages bounds checking by > > > replacing direct modification of the rq_respages and rq_next_page > > > fields with accessor functions. > > > > > > --- > > > > > > Chuck Lever (2): > > > SUNRPC: add bounds checking to svc_rqst_replace_page > > > NFSD: Watch for rq_pages bounds checking errors in nfsd_splice_actor() > > > > > > Jeff Layton (1): > > > nfsd: don't replace page in rq_pages if it's a continuation of last page > > > > > > > > > fs/nfsd/vfs.c | 15 +++++++++++++-- > > > include/linux/sunrpc/svc.h | 2 +- > > > include/trace/events/sunrpc.h | 25 +++++++++++++++++++++++++ > > > net/sunrpc/svc.c | 15 ++++++++++++++- > > > 4 files changed, 53 insertions(+), 4 deletions(-) > > > > > > -- > > > Chuck Lever > > > > > > > Looks good, Chuck, thanks. You can add this to the last two: > > > > Reviewed-by: Jeff Layton <jlayton@kernel.org> > > Excellent, thanks! > > When I started I expected 3/3 to be more substantial, but since it's > just a handful of lines and the patch descriptions are about the same, > I'm going to squash 2/3 and 3/3 together. > > Only question is whether to apply that to nfsd-next or nfsd-fixes. > Since it's a defensive change, I was thinking nfsd-next. Let me know > if you think it should get merged sooner. > No, that sounds fine for those. Patch #1 needs to go in ASAP, of course.
A slightly modified take on Jeff's earlier patches, tested with both NFSv3 and NFSv4.1 via simple fault injection in svc_rqst_replace_page(). In general I'm in favor of more rq_pages bounds checking by replacing direct modification of the rq_respages and rq_next_page fields with accessor functions. --- Chuck Lever (2): SUNRPC: add bounds checking to svc_rqst_replace_page NFSD: Watch for rq_pages bounds checking errors in nfsd_splice_actor() Jeff Layton (1): nfsd: don't replace page in rq_pages if it's a continuation of last page fs/nfsd/vfs.c | 15 +++++++++++++-- include/linux/sunrpc/svc.h | 2 +- include/trace/events/sunrpc.h | 25 +++++++++++++++++++++++++ net/sunrpc/svc.c | 15 ++++++++++++++- 4 files changed, 53 insertions(+), 4 deletions(-) -- Chuck Lever