{184324980} Add bounds check for legacy request buffers#5958
Conversation
4cb0175 to
81751e2
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
logfill [db unavailable at finish] **quarantined**
sc_timepart_multiddl_generated
consumer_non_atomic_default_consumer_generated **quarantined**
| hdr.opcode = 255; // invalid opcode | ||
| sz = sizeof(hdr); | ||
| memcpy(p_slock->bigbuf, &hdr, sz); | ||
| // application should get a 199 (ERR_BADREQ) |
There was a problem hiding this comment.
Should buf be made to point to p_slock->bigbuf too? Or point to &hdr even? I asked because buf still gets used below (e.g., in line 525, the req_hdr_get call) and I find it a bit confusing that it's pointing to data of invalid size - even if sz invariably addresses the bound checking issue itself.
There was a problem hiding this comment.
Sure. It can be set for consistency.
Signed-off-by: Mike Ponomarenko <mponomarenko@bloomberg.net>
81751e2 to
5934e9e
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
sc_truncate [db unavailable at finish]
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**
No description provided.