[ 
https://issues.apache.org/jira/browse/KUDU-3450?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18088125#comment-18088125
 ] 

ASF subversion and git services commented on KUDU-3450:
-------------------------------------------------------

Commit 55726fdfa77ff2b1d59d0f60b61be5597555dca0 in kudu's branch 
refs/heads/branch-1.17.x from Ashwani
[ https://gitbox.apache.org/repos/asf?p=kudu.git;h=55726fdfa ]

[subprocess] Fix buffer over-read/overflow in message discard

SubprocessProtocol::DoReadAndDiscard() clears an oversized message off
the communication channel using a fixed 4K stack buffer, but bounded
each read() with std::max<ssize_t>(rem, sizeof(buf)) instead of
std::min(). Since this returns the greater of the two, the read length
is never held to the buffer size. This causes two issues:
* Stack buffer overflow: when the discarded payload is larger than
  4K (say, 8K message), a single read() is asked to copy up to 'rem'
  bytes into the 4K buffer, smashing the stack.
* Valid message subsumption: when the oversized payload is smaller than
  4K, the read still pulls up to 4K, over-reading into and consuming the
  following well-formed message. The next ReceiveMessage() then loses
  that message, breaking the "discard to re-sync the channel" guarantee.

The bug has existed since DoReadAndDiscard() was introduced in KUDU-3450.
The patch fixes it by limiting each read to std::min(rem, sizeof(buf)),
matching the Java implementation in doReadAndDiscard.

Add two unit tests covering both scenarios: a deterministic over-read
and an oversized-payload overflow test.

Change-Id: I73da816132555cb3d29b7bbf8a6821ad3f2dec9c
Reviewed-on: http://gerrit.cloudera.org:8080/24427
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
(cherry picked from commit c968d85cc9d5ea18cbabf7464cb48bbec51a63a6)
Reviewed-on: http://gerrit.cloudera.org:8080/24433
Reviewed-by: Ashwani Raina <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>


> SubprocessProtocol has a hard-coded limit on message size, but RangerClient 
> doesn't honor that while generating requests
> ------------------------------------------------------------------------------------------------------------------------
>
>                 Key: KUDU-3450
>                 URL: https://issues.apache.org/jira/browse/KUDU-3450
>             Project: Kudu
>          Issue Type: Bug
>          Components: subprocess
>    Affects Versions: 1.12.0, 1.13.0, 1.14.0, 1.15.0, 1.16.0
>            Reporter: Alexey Serbin
>            Priority: Major
>             Fix For: 1.17.0
>
>
> As one can see from the code in the 
> [SubprocessProtocol|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/src/kudu/subprocess/subprocess_protocol.cc#L106-L110]
>  and in [MessageIO utility Java 
> class|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java#L69-L73],
>  it's not possible to receive messages in PB format that are larger than a 
> pre-defined limit.  Also, there isn't a way to pass 
> {{SubprocessRequestPB::request}} and {{SubprocessResponsePB::response}} data 
> in multiple chunks in the SubprocessProtocol by design.
> The default setting for the maximum message size is 1MiByte in the 
> implementation of [the relevant 
> C++|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/src/kudu/subprocess/subprocess_protocol.cc#L46]
>  and [Java subprocess 
> components|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java#L54].
> The implementation of the related C++ components has the setting effectively 
> hard-coded and there isn't a way to change it.   The implementation of the 
> related Java components has this setting configurable via the {{maxMsgBytes}} 
> configuration property.
> Anyways, nether the code in {{RangerClient}} nor the code in 
> {{RangerProtocolHandler}} honors the limit:
> * 
> [RangerClient::AuthorizeActionMultipleColumns()|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/src/kudu/ranger/ranger_client.cc#L435-L448]
> * 
> [RangerClient::AuthorizeActionMultipleTables()|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/src/kudu/ranger/ranger_client.cc#L468-L492]
> * 
> [ProtocolHandler.unpackAndExecuteRequest()|https://github.com/apache/kudu/blob/9684200713b5e1bf258437527127bd98acfa1e42/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ProtocolHandler.java#L47-L56]
> With that, it's possible to get into an impasse situation from the both 
> sides.  For example, if a large request has been generated by 
> {{RangerClient::AuthorizeActionMultipleColumns()}} or 
> {{RangerClient::AuthorizeActionMultipleTables()}}, the following stack trace 
> might be observed in the output of the subprocess that runs the Ranger client 
> plugin:
> {noformat}
> Exception in thread "main" java.util.concurrent.CompletionException: 
> org.apache.kudu.subprocess.KuduSubprocessException: Unable to read the 
> protobuf message
>         at 
> java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:273)
>         at 
> java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:280)
>         at 
> java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1629)
>         at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at java.lang.Thread.run(Thread.java:748)
> Caused by: org.apache.kudu.subprocess.KuduSubprocessException: Unable to read 
> the protobuf message
>         at org.apache.kudu.subprocess.MessageReader.run(MessageReader.java:74)
>         at 
> java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1626)
>         ... 3 more
> Caused by: java.io.IOException: message size (2763197) exceeds maximum 
> message size (1048576)
>         at org.apache.kudu.subprocess.MessageIO.readBytes(MessageIO.java:71)
>         at org.apache.kudu.subprocess.MessageReader.run(MessageReader.java:68)
>         ... 4 more
> {noformat}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to