Page MenuHomeFreeBSD
Differential D29752

sbuf_uionew(): sbuf_new() takes int as length
ClosedPublic
Actions

Authored by kib on Apr 13 2021, 7:15 PM.
Tags
None
Referenced Files
F106100926: D29752.diff
Wed, Dec 25, 10:12 AM
Unknown Object (File)
Sat, Dec 21, 4:16 PM
Unknown Object (File)
Tue, Dec 10, 7:08 AM
Unknown Object (File)
Oct 17 2024, 9:53 PM
Unknown Object (File)
Oct 17 2024, 9:53 PM
Unknown Object (File)
Oct 17 2024, 9:53 PM
Unknown Object (File)
Oct 17 2024, 9:53 PM
Unknown Object (File)
Oct 17 2024, 9:52 PM
View All 77 Files
Subscribers
pho

Details

Reviewers
markj
Commits
rG5edf7227ec33: pseudofs: limit writes to 1M
rG116f26f947b8: sbuf_uionew(): sbuf_new() takes int as length
Summary

Noted by: markj

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Apr 13 2021, 7:15 PM
kib created this revision.
markj accepted this revision.Apr 13 2021, 7:38 PM
markj added inline comments.
sys/kern/subr_sbuf.c
272

This still permits userspace to trigger a large kernel allocation.

pfs_read() imposes a limit of 1MB, PFS_MAXBUFSIZ. I think this is probably a reasonable limit for writes as well?

This revision is now accepted and ready to land.Apr 13 2021, 7:38 PM
kib updated this revision to Diff 87395.Apr 13 2021, 7:42 PM

Also check against SBUF_MINSIZE

This revision now requires review to proceed.Apr 13 2021, 7:42 PM
kib marked an inline comment as done.Apr 13 2021, 7:49 PM
kib added inline comments.
sys/kern/subr_sbuf.c
272

Even 1M is arguably large kernel allocation.
Without drain, it is the only way around.

I do not want to add drain support and drain code.

kib updated this revision to Diff 87396.Apr 13 2021, 7:50 PM
kib marked an inline comment as done.

Limit pseudofs max write size to 1M

markj added inline comments.Apr 13 2021, 8:15 PM
sys/kern/subr_sbuf.c
269

I suspect it should be uio->uio_resid < SBUF_MINSIZE - 1, otherwise it is impossible to write to a 1-byte file.

272

And consumers have to opt in to using sbuf drain anyway.

kib updated this revision to Diff 87397.Apr 13 2021, 8:25 PM
kib marked an inline comment as done.

SBUF_MAXSIZE - 1

markj accepted this revision.Apr 13 2021, 8:29 PM
This revision is now accepted and ready to land.Apr 13 2021, 8:29 PM
pho added a comment.Apr 14 2021, 7:08 AM

D29752.87397.diff looks good to me. Tested with syscall fuzzing of:

# /* 174 is freebsd6 pwrite */
#define SYS_pwritev     290
#define SYS_pwrite      476
#define SYS_preadv      289
Closed by commit rG116f26f947b8: sbuf_uionew(): sbuf_new() takes int as length (authored by kib). · Explain WhyApr 14 2021, 7:25 AM
This revision was automatically updated to reflect the committed changes.
kib added a commit: rG116f26f947b8: sbuf_uionew(): sbuf_new() takes int as length.
kib added a commit: rG5edf7227ec33: pseudofs: limit writes to 1M.

Revision Contents
Changeset List

PathSize
sys/
kern/
4 lines

Diff 87421

View Options

sys/kern/subr_sbuf.c

Loading...