9669 Extra zeros sent by sendfile()
Review Request #1141 - Created July 24, 2018 and submitted
Information | |
---|---|
Marcel Telka | |
illumos-gate | |
master | |
9669 | |
451cf09... | |
Reviewers | |
general | |
This fixes a bug in the sendfile(3ext)/sendfilev(3ext) implementation where some extra zeros could be written to output in a case we asked to copy more data than there are in the input file.
Without the fix --------------- # dtrace -n 'tcp_zcopy_check:return /pid==$target/ {trace(arg1)} snf_segmap:entry /pid==$target/ {stack();trace(arg3)}' -c ./test-64 dtrace: description 'tcp_zcopy_check:return ' matched 2 probes sendfile call: off = 0, len = 8192 sendfile: ret = 8192, off = 8192, errno = 0 sendfile call: off = 20, len = 8192 sendfile: ret = 8192, off = 8212, errno = 0 dtrace: pid 675 has exited CPU ID FUNCTION:NAME 0 57683 tcp_zcopy_check:return 1 0 59899 snf_segmap:entry genunix`sendvec_chunk+0xb94 genunix`sendfilev+0x36d unix`sys_syscall+0x177 8192 0 59899 snf_segmap:entry genunix`sendvec_chunk+0xb94 genunix`sendfilev+0x36d unix`sys_syscall+0x177 8192 # With the fix ------------ # dtrace -n 'tcp_zcopy_check:return /pid==$target/ {trace(arg1)} snf_segmap:entry /pid==$target/ {stack();trace(arg3)}' -c ./test-64 dtrace: description 'tcp_zcopy_check:return ' matched 2 probes sendfile call: off = 0, len = 8192 sendfile: ret = 6, off = 6, errno = 0 sendfile call: off = 20, len = 8192 sendfile: ret = 0, off = 20, errno = 0 dtrace: pid 100698 has exited CPU ID FUNCTION:NAME 0 49879 tcp_zcopy_check:return 1 0 52107 snf_segmap:entry genunix`sendvec_chunk+0xbe3 genunix`sendfilev+0x3ba unix`sys_syscall+0x27d 6 #
could you also post the test-64 source in testing done or is it part of some test suite already?
-
usr/src/uts/common/syscall/sendfile.c (Diff revision 1) -
Is there a reason that we're performing this check outside of snf_segmap? Isn't this something that all callers of snf_segmap need to do and therefore we should not be sending data beyond that which we can read? Given that we have checks in snf_segmap that seem to adjust the total size, is there a reason that we're not leveraging those and cleaning them up versus asking for a different length at this point? Does sosendfile64 suffer from the same problem as it also calls snf_segmap?
Review request changed
Change Summary:
Added comment
to snf_segmap()
(+ nits).
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+34 -31) |
Ship It!
Review request changed
Status: Closed (submitted)
Change Summary:
commit 8cd3131235b232e4d63be3cf95ce9be87907e74f Author: Marcel Telka <marcel@telka.sk> AuthorDate: Wed Jul 25 12:06:18 2018 +0200 Commit: Richard Lowe <richlowe@richlowe.net> CommitDate: Mon Aug 6 19:38:36 2018 +0000 9669 Extra zeros sent by sendfile() Reviewed by: Robert Mustacchi <rm@joyent.com> Approved by: Richard Lowe <richlowe@richlowe.net> :100644 100644 21f3744895 4cbd079539 M usr/src/uts/common/fs/sockfs/socksyscalls.c :100644 100644 822f14416c 0cfafbf13f M usr/src/uts/common/syscall/sendfile.c