Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INKVConnInternal::do_io_*: handle null buffer #11789

Merged

Conversation

bneradt
Copy link
Contributor

@bneradt bneradt commented Sep 24, 2024

It's common for users of VC's to cancel io via a 0 byte, nullptr read or write on the VC. INKVConnInternal::do_io_read and
INKVConnInternal::do_io_write were not prepared to handle such cancellations. This updates them to handle this gracefully rather than crashing on a nullptr dereference. This change was found to be needed for the multiplexer plugin for handling HttpTunnel aborts.

For reference, see, for example, UnixNetVConnection::do_io_read which handles a nullptr MIOBuffer. This basically copies that logic into INKVConnInternal so it handles cancellation gracefully.


For Review

Hiding whitespace when viewing the diff makes this patch more understandable.

It's common for users of VC's to cancel io via a 0 byte, nullptr read or
write on the VC. INKVConnInternal::do_io_read and
INKVConnInternal::do_io_write were not prepared to handle such
cancellations. This updates them to handle this gracefully rather than
crashing on a nullptr dereference. This change is was found to be needed
for the multiplexer plugin for handling HttpTunnel aborts.

For reference, see, for example, UnixNetVConnection::do_io_read which
handles a nullptr MIOBuffer. This basically copies that logic into
INKVConnInternal so it handles cancellation gracefully.
@bneradt bneradt added the Core label Sep 24, 2024
@bneradt bneradt added this to the 10.1.0 milestone Sep 24, 2024
@bneradt bneradt self-assigned this Sep 24, 2024
@bneradt bneradt added the Crash label Sep 24, 2024
Copy link
Contributor

@cmcfarlen cmcfarlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, makes sense! Thanks!

@bneradt bneradt merged commit b848a2e into apache:master Oct 1, 2024
15 checks passed
@bneradt bneradt deleted the fix_inkvconninternal_do_io_for_canceled_io branch October 1, 2024 19:23
bneradt pushed a commit to bneradt/trafficserver that referenced this pull request Oct 1, 2024
It's common for users of VC's to cancel io via a 0 byte, nullptr read or
write on the VC. INKVConnInternal::do_io_read and
INKVConnInternal::do_io_write were not prepared to handle such
cancellations. This updates them to handle this gracefully rather than
crashing on a nullptr dereference. This change is was found to be needed
for the multiplexer plugin for handling HttpTunnel aborts.

For reference, see, for example, UnixNetVConnection::do_io_read which
handles a nullptr MIOBuffer. This basically copies that logic into
INKVConnInternal so it handles cancellation gracefully.
@bneradt
Copy link
Contributor Author

bneradt commented Oct 1, 2024

This didn't backport cleanly to 10.0.x, so I created a separate PR for it:
#11802

cmcfarlen pushed a commit that referenced this pull request Oct 1, 2024
It's common for users of VC's to cancel io via a 0 byte, nullptr read or
write on the VC. INKVConnInternal::do_io_read and
INKVConnInternal::do_io_write were not prepared to handle such
cancellations. This updates them to handle this gracefully rather than
crashing on a nullptr dereference. This change is was found to be needed
for the multiplexer plugin for handling HttpTunnel aborts.

For reference, see, for example, UnixNetVConnection::do_io_read which
handles a nullptr MIOBuffer. This basically copies that logic into
INKVConnInternal so it handles cancellation gracefully.

Co-authored-by: bneradt <bneradt@yahooinc.com>
bneradt added a commit to bneradt/trafficserver that referenced this pull request Nov 13, 2024
This is a very similar change as apache#11789. With this patch,
TransformTerminus properly supports a 0 byte read to cancel VIO. Without
this patch, the logic naively scheduled an event which called the
handler which would crash on what was often a canceled continuation or
otherwise invalid continuation.
bneradt added a commit to bneradt/trafficserver that referenced this pull request Nov 26, 2024
This is a very similar change as apache#11789. With this patch,
TransformTerminus properly supports a 0 byte read to cancel VIO. Without
this patch, the logic naively scheduled an event which called the
handler which would crash on what was often a canceled continuation or
otherwise invalid continuation.
bneradt pushed a commit to bneradt/trafficserver that referenced this pull request Nov 26, 2024
)

It's common for users of VC's to cancel io via a 0 byte, nullptr read or
write on the VC. INKVConnInternal::do_io_read and
INKVConnInternal::do_io_write were not prepared to handle such
cancellations. This updates them to handle this gracefully rather than
crashing on a nullptr dereference. This change is was found to be needed
for the multiplexer plugin for handling HttpTunnel aborts.

For reference, see, for example, UnixNetVConnection::do_io_read which
handles a nullptr MIOBuffer. This basically copies that logic into
INKVConnInternal so it handles cancellation gracefully.

(cherry picked from commit b848a2e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants