summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaichi Hirono2016-12-05 01:50:44 -0600
committerDaichi Hirono2016-12-11 23:34:29 -0600
commita6373ec1d47c0b370c87b3915feeba8f2b4523f7 (patch)
treed8e532d25edd551db366b825e96f22a69faf8b48 /libappfuse
parent4b4475308598659137657261218fc5a3207ec79c (diff)
downloadplatform-system-core-a6373ec1d47c0b370c87b3915feeba8f2b4523f7.tar.gz
platform-system-core-a6373ec1d47c0b370c87b3915feeba8f2b4523f7.tar.xz
platform-system-core-a6373ec1d47c0b370c87b3915feeba8f2b4523f7.zip
Fix checks for reading and writing FuseMessage.
Previously FuseMessage were checking result of read/write operation after checking header.len value is valid. This was wrong because header.len does not contain correct value when read function does not read any bytes and returns zero. Bug: 33278098 Test: libappfuse_test Change-Id: Icf998ca6c3eeee20cbc4aa2f65195a87e59ffc27
Diffstat (limited to 'libappfuse')
-rw-r--r--libappfuse/FuseBuffer.cc37
-rw-r--r--libappfuse/include/libappfuse/FuseBuffer.h3
2 files changed, 27 insertions, 13 deletions
diff --git a/libappfuse/FuseBuffer.cc b/libappfuse/FuseBuffer.cc
index 3ade31c54..882d54552 100644
--- a/libappfuse/FuseBuffer.cc
+++ b/libappfuse/FuseBuffer.cc
@@ -34,26 +34,38 @@ static_assert(
34 "FuseBuffer must be standard layout union."); 34 "FuseBuffer must be standard layout union.");
35 35
36template <typename T> 36template <typename T>
37bool FuseMessage<T>::CheckHeaderLength() const { 37bool FuseMessage<T>::CheckPacketSize(size_t size, const char* name) const {
38 const auto& header = static_cast<const T*>(this)->header; 38 const auto& header = static_cast<const T*>(this)->header;
39 if (sizeof(header) <= header.len && header.len <= sizeof(T)) { 39 if (size >= sizeof(header) && size <= sizeof(T)) {
40 return true; 40 return true;
41 } else { 41 } else {
42 LOG(ERROR) << "Packet size is invalid=" << header.len; 42 LOG(ERROR) << name << " is invalid=" << size;
43 return false; 43 return false;
44 } 44 }
45} 45}
46 46
47template <typename T> 47template <typename T>
48bool FuseMessage<T>::CheckResult( 48bool FuseMessage<T>::CheckResult(int result, const char* operation_name) const {
49 int result, const char* operation_name) const { 49 if (result == 0) {
50 // Expected close of other endpoints.
51 return false;
52 }
53 if (result < 0) {
54 PLOG(ERROR) << "Failed to " << operation_name << " a packet";
55 return false;
56 }
57 return true;
58}
59
60template <typename T>
61bool FuseMessage<T>::CheckHeaderLength(int result, const char* operation_name) const {
50 const auto& header = static_cast<const T*>(this)->header; 62 const auto& header = static_cast<const T*>(this)->header;
51 if (result >= 0 && static_cast<uint32_t>(result) == header.len) { 63 if (static_cast<uint32_t>(result) == header.len) {
52 return true; 64 return true;
53 } else { 65 } else {
54 PLOG(ERROR) << "Failed to " << operation_name 66 LOG(ERROR) << "Invalid header length: operation_name=" << operation_name
55 << " a packet. result=" << result << " header.len=" 67 << " result=" << result
56 << header.len; 68 << " header.len=" << header.len;
57 return false; 69 return false;
58 } 70 }
59} 71}
@@ -61,17 +73,18 @@ bool FuseMessage<T>::CheckResult(
61template <typename T> 73template <typename T>
62bool FuseMessage<T>::Read(int fd) { 74bool FuseMessage<T>::Read(int fd) {
63 const ssize_t result = TEMP_FAILURE_RETRY(::read(fd, this, sizeof(T))); 75 const ssize_t result = TEMP_FAILURE_RETRY(::read(fd, this, sizeof(T)));
64 return CheckHeaderLength() && CheckResult(result, "read"); 76 return CheckResult(result, "read") && CheckPacketSize(result, "read count") &&
77 CheckHeaderLength(result, "read");
65} 78}
66 79
67template <typename T> 80template <typename T>
68bool FuseMessage<T>::Write(int fd) const { 81bool FuseMessage<T>::Write(int fd) const {
69 const auto& header = static_cast<const T*>(this)->header; 82 const auto& header = static_cast<const T*>(this)->header;
70 if (!CheckHeaderLength()) { 83 if (!CheckPacketSize(header.len, "header.len")) {
71 return false; 84 return false;
72 } 85 }
73 const ssize_t result = TEMP_FAILURE_RETRY(::write(fd, this, header.len)); 86 const ssize_t result = TEMP_FAILURE_RETRY(::write(fd, this, header.len));
74 return CheckResult(result, "write"); 87 return CheckResult(result, "write") && CheckHeaderLength(result, "write");
75} 88}
76 89
77template class FuseMessage<FuseRequest>; 90template class FuseMessage<FuseRequest>;
diff --git a/libappfuse/include/libappfuse/FuseBuffer.h b/libappfuse/include/libappfuse/FuseBuffer.h
index e7f620cb6..276db9020 100644
--- a/libappfuse/include/libappfuse/FuseBuffer.h
+++ b/libappfuse/include/libappfuse/FuseBuffer.h
@@ -34,8 +34,9 @@ class FuseMessage {
34 bool Read(int fd); 34 bool Read(int fd);
35 bool Write(int fd) const; 35 bool Write(int fd) const;
36 private: 36 private:
37 bool CheckHeaderLength() const; 37 bool CheckPacketSize(size_t size, const char* name) const;
38 bool CheckResult(int result, const char* operation_name) const; 38 bool CheckResult(int result, const char* operation_name) const;
39 bool CheckHeaderLength(int result, const char* operation_name) const;
39}; 40};
40 41
41// FuseRequest represents file operation requests from /dev/fuse. It starts 42// FuseRequest represents file operation requests from /dev/fuse. It starts