Skip to content

Commit 1aa76a8

Browse files
committed
mrp: Fix unreachable padding code in MrpTlvSerializer::deserialize()
Every case in the switch returned directly, making the padding reading code after the switch unreachable. Since the serialize side writes 4-byte alignment padding for each TLV (except END), the deserialize side must also read it. Restructure to use break instead of return in each case, with a shared result variable and common padding/return at the end. The END case still returns early as it has no padding.
1 parent b14e074 commit 1aa76a8

1 file changed

Lines changed: 24 additions & 14 deletions

File tree

src/inet/linklayer/mrp/MrpPduSerializer.cc

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
166166
size_t startPos = stream.getPosition().get<B>();
167167
auto headerType = static_cast<TlvHeaderType>(stream.readUint8());
168168
uint8_t headerLength = stream.readUint8();
169+
Ptr<MrpTlvHeader> tlvReturnValue;
169170

170171
switch (headerType) {
171172
case END: {
@@ -181,7 +182,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
181182
tlv->setSequenceID(stream.readUint16Be());
182183
tlv->setUuid0(stream.readUint64Be());
183184
tlv->setUuid1(stream.readUint64Be());
184-
return tlv;
185+
tlvReturnValue = tlv;
186+
break;
185187
}
186188
case TEST: {
187189
auto tlv = makeShared<MrpTest>();
@@ -193,7 +195,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
193195
tlv->setRingState(stream.readUint16Be());
194196
tlv->setTransition(stream.readUint16Be());
195197
tlv->setTimeStamp(stream.readUint32Be());
196-
return tlv;
198+
tlvReturnValue = tlv;
199+
break;
197200
}
198201
case TOPOLOGYCHANGE: {
199202
auto tlv = makeShared<MrpTopologyChange>();
@@ -202,7 +205,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
202205
tlv->setPrio(stream.readUint16Be());
203206
tlv->setSa(stream.readMacAddress());
204207
tlv->setInterval(stream.readUint16Be());
205-
return tlv;
208+
tlvReturnValue = tlv;
209+
break;
206210
}
207211
case LINKDOWN:
208212
case LINKUP: {
@@ -212,7 +216,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
212216
tlv->setSa(stream.readMacAddress());
213217
tlv->setInterval(stream.readUint16Be());
214218
tlv->setBlocked(stream.readUint16Be());
215-
return tlv;
219+
tlvReturnValue = tlv;
220+
break;
216221
}
217222
case INTEST: {
218223
auto tlv = makeShared<MrpInTest>();
@@ -224,7 +229,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
224229
tlv->setInState(stream.readUint16Be());
225230
tlv->setTransition(stream.readUint16Be());
226231
tlv->setTimeStamp(stream.readUint32Be());
227-
return tlv;
232+
tlvReturnValue = tlv;
233+
break;
228234
}
229235
case INTOPOLOGYCHANGE: {
230236
auto tlv = makeShared<MrpInTopologyChange>();
@@ -233,7 +239,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
233239
tlv->setSa(stream.readMacAddress());
234240
tlv->setInID(stream.readUint16Be());
235241
tlv->setInterval(stream.readUint16Be());
236-
return tlv;
242+
tlvReturnValue = tlv;
243+
break;
237244
}
238245
case INLINKDOWN:
239246
case INLINKUP: {
@@ -245,7 +252,8 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
245252
tlv->setInID(stream.readUint16Be());
246253
tlv->setInterval(stream.readUint16Be());
247254
tlv->setLinkInfo(stream.readUint16Be());
248-
return tlv;
255+
tlvReturnValue = tlv;
256+
break;
249257
}
250258
case INLINKSTATUSPOLL: {
251259
auto tlv = makeShared<MrpInLinkStatusPoll>();
@@ -254,26 +262,28 @@ const Ptr<Chunk> MrpTlvSerializer::deserialize(MemoryInputStream &stream) const
254262
tlv->setSa(stream.readMacAddress());
255263
tlv->setPortRole(stream.readUint16Be());
256264
tlv->setInID(stream.readUint16Be());
257-
return tlv;
265+
tlvReturnValue = tlv;
266+
break;
258267
}
259268
case OPTION: {
260269
auto tlv = makeShared<MrpOption>();
261270
tlv->setHeaderType(headerType);
262271
tlv->setValueLength(headerLength);
263272
tlv->setOuiType(static_cast<MrpOuiType>(stream.readUint24Be()));
264273
tlv->setEd1Type(stream.readUint8());
265-
return tlv;
274+
tlvReturnValue = tlv;
275+
break;
266276
}
267277
default:
268278
throw cRuntimeError("Unknown Header TYPE value: %d",
269279
static_cast<int>(headerType));
270280
}
271281

272-
if (headerType != END) {
273-
size_t length = stream.getPosition().get<B>() - startPos;
274-
size_t paddingLength = (4 - (length % 4)) % 4;
275-
stream.readByteRepeatedly(0, paddingLength);
276-
}
282+
size_t length = stream.getPosition().get<B>() - startPos;
283+
size_t paddingLength = (4 - (length % 4)) % 4;
284+
stream.readByteRepeatedly(0, paddingLength);
285+
286+
return tlvReturnValue;
277287
}
278288

279289
const Ptr<Chunk> MrpSubTlvSerializer::deserialize(MemoryInputStream &stream) const

0 commit comments

Comments
 (0)