Skip to content

Commit 126a29c

Browse files
MichaelCuevasfacebook-github-bot
authored andcommitted
PrivHelper: fix two [de]serialization bugs
Summary: # This diff Fixes some [de]serialization logic in the PrivHelper. The two bugs were: 1) We were previously [de]serializing some values as uint32_t instead of int32_t 2) We were returning an uint64_t when deserializing a uint32_t. This caused us to potentially "narrow"/truncate values when assigning the result to a variable that's smaller than uint64_t. I have convinced myself that this should be safe, since: 1) For problem #1, we don't use signed values anywhere, so the change from uint32_t to int32_t should be no-op. Additionally, all the values we use are sufficiently small such that uint32_t -> int32_t shouldn't cause overflow. 2) For problem #2, the deserialized values should always be <= MAX_UINT32 because we only accept uint32_t values when serializing. In addition, no users of the function currently expect a uint64_t to be returned (they all accidentally truncate the value). Therefore, the change to the type should be no-op. Reviewed By: genevievehelsel Differential Revision: D77240439 fbshipit-source-id: 90a4c49a1bde8428e4a4c50c5665c4889743eeaa
1 parent 3225468 commit 126a29c

File tree

1 file changed

+13
-5
lines changed

1 file changed

+13
-5
lines changed

eden/fs/privhelper/PrivHelperConn.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,18 @@ void serializeUint32(Appender& a, uint64_t val) {
129129
a.write<uint32_t>(val);
130130
}
131131

132-
uint64_t deserializeUint32(Cursor& cursor) {
132+
uint32_t deserializeUint32(Cursor& cursor) {
133133
return cursor.read<uint32_t>();
134134
}
135135

136+
void serializeInt32(Appender& a, int32_t val) {
137+
a.write<int32_t>(val);
138+
}
139+
140+
int32_t deserializeInt32(Cursor& cursor) {
141+
return cursor.read<int32_t>();
142+
}
143+
136144
void serializeSocketAddress(Appender& a, const folly::SocketAddress& addr) {
137145
bool isInet = addr.isFamilyInet();
138146
serializeBool(a, isInet);
@@ -168,9 +176,9 @@ void serializeNFSMountOptions(Appender& a, const NFSMountOptions& options) {
168176
serializeUint32(a, options.writeIOSize);
169177
serializeOption(a, options.directoryReadSize);
170178
serializeUint8(a, options.readAheadSize);
171-
serializeUint32(a, options.retransmitTimeoutTenthSeconds);
179+
serializeInt32(a, options.retransmitTimeoutTenthSeconds);
172180
serializeUint32(a, options.retransmitAttempts);
173-
serializeUint32(a, options.deadTimeoutSeconds);
181+
serializeInt32(a, options.deadTimeoutSeconds);
174182
serializeOption(a, options.dumbtimer);
175183
}
176184

@@ -186,9 +194,9 @@ NFSMountOptions deserializeNFSMountOptions(Cursor& cursor) {
186194
options.writeIOSize = deserializeUint32(cursor);
187195
options.directoryReadSize = deserializeOption<uint32_t>(cursor);
188196
options.readAheadSize = deserializeUint8(cursor);
189-
options.retransmitTimeoutTenthSeconds = deserializeUint32(cursor);
197+
options.retransmitTimeoutTenthSeconds = deserializeInt32(cursor);
190198
options.retransmitAttempts = deserializeUint32(cursor);
191-
options.deadTimeoutSeconds = deserializeUint32(cursor);
199+
options.deadTimeoutSeconds = deserializeInt32(cursor);
192200
options.dumbtimer = deserializeOption<bool>(cursor);
193201
return options;
194202
}

0 commit comments

Comments
 (0)