TSUserArg: add value type checking #8550
Conversation
|
Nitpick: seems like static constexpr items should be private members of PluginUserArgsMixin. But that's perhaps beyond the scope of this PR. |
I agree that it's good to keep stuff private if they can be, but these functions are both needed externally in the InkAPI.cc code. Here, for example: |
bneradt
force-pushed
the
make_user_args_type_safer
branch
from
b120782 to
24dcc2b
Compare
|
[approve ci autest] |
|
Unfortunately I think this is probably detecting errors in core plugins, that you'll need to fix in this PR to get Au test regression to pass. I suggest trying the failing cases in your own test environment, see what's going on. |
|
I like this |
Correct, there were issues with core plugins. But I think those are now both addressed and merged in via the PRs mentioned in the description. We'll see whether I missed any with this latest autest run. |
bneradt
force-pushed
the
make_user_args_type_safer
branch
from
24dcc2b to
50e66bb
Compare
A plugin may have multiple user argument types. For instance, it may have both TS_USER_ARGS_SSN and TS_USER_ARGS_TXN plugin data. Due to a coding error, it is possible that it may accidentally use the wrong index type while retrieving one or the other user data types. For instance, if it uses its transaction index to retrieve its session data, this may result in them getting the transaction data for another plugin. In these situations, the plugin will either read what looks like garbage data to it (because it is casting incorrect memory to its data type), or it may write to that data and thus corrupt the other plugin's data, or both. This change updates the core to segment off the plugin user data ids by category. This allows the core to verify that the plugin passes the correct id type per the data type (session, transaction, etc.) and fails an assertion if there is a mismatch.
bneradt
force-pushed
the
make_user_args_type_safer
branch
from
50e66bb to
9a0da36
Compare
|
@masaori335 has volunteered to review this. |
|
I'd like to leave this out of 9.1.x if ok with you. |
A plugin may have multiple user argument types. For instance, it may have both TS_USER_ARGS_SSN and TS_USER_ARGS_TXN plugin data. Due to a coding error, it is possible that it may accidentally use the wrong index type while retrieving one or the other user data types. For instance, if it uses its transaction index to retrieve its session data, this may result in them getting the transaction data for another plugin. In these situations, the plugin will either read what looks like garbage data to it (because it is casting incorrect memory to its data type), or it may write to that data and thus corrupt the other plugin's data, or both. This change updates the core to segment off the plugin user data ids by category. This allows the core to verify that the plugin passes the correct id type per the data type (session, transaction, etc.) and fails an assertion if there is a mismatch. (cherry picked from commit 5774921)
|
Cherry-picked to v9.2.x |
Fine with me. I'll remove the 9.1.x project. |
* asf/9.2.x: Updated ChangeLog docs: fix fedora install notes and spelling issues (apache#8537) Docs: Fix default value of proxy.config.ssl.handshake_timeout_in (apache#8574) Partial of revert "Cleanup generated LDFLAGS for jemalloc (apache#8285)" (apache#8533) TSUserArg: add value type checking (apache#8550) Relax key validation of sni.yaml (apache#8549) Clear random header value by AIO read error (apache#8559) Fixes macOS arm64 builds (again) (apache#8556) Traffic Dump: Use the correct transaction user index (apache#8548) combo_handler: Initialize User Arg Index in TSRemapInit (apache#8551) backout down parent retry limiting in parent selection and nexthop (apache#8546)
5 participants
A plugin may have multiple user argument types. For instance, it may
have both TS_USER_ARGS_SSN and TS_USER_ARGS_TXN plugin data. Due to a
coding error, it is possible that it may accidentally use the wrong
index type while retrieving one or the other user data types. For
instance, if it uses its transaction index to retrieve its session data,
this may result in them getting the transaction data for another plugin.
In these situations, the plugin will either read what looks like garbage
data to it (because it is casting incorrect memory to its data type), or
it may write to that data and thus corrupt the other plugin's data, or
both.
This change updates the core to segment off the plugin user data ids by
category. This allows the core to verify that the plugin passes the
correct id type per the data type (session, transaction, etc.) and
fails an assertion if there is a mismatch.
Note: autests for this PR will fail until a couple plugins with indexing issues uncovered by this patch are merged in: