On 07/11/2018 04:04 AM, jiangyiwen wrote: > On 2018/7/10 3:26, Tomas Bortoli wrote: >> The pdu_read() function suffers from an integer underflow. >> When pdu->offset is greater than pdu->size, the length calculation will have >> a wrong result, resulting in an out-of-bound read. >> This patch modifies also pdu_write() in the same way to prevent the same >> issue from happening there and for consistency. > I guess this case may happened only when server send wrong size to > the client and then cause size < offset, or else I think this case > will not happen. Is it right? Or other cases? > > In addition, the email should also send to andrew morton, because > 9p maintainer already don't maintain the project, andrew can help > merge the patch. > >> Signed-off-by: Tomas Bortoli <[email protected]> >> Reported-by: [email protected] >> --- >> net/9p/protocol.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/net/9p/protocol.c b/net/9p/protocol.c >> index 931ea00c4fed..f1e2425f920b 100644 >> --- a/net/9p/protocol.c >> +++ b/net/9p/protocol.c >> @@ -55,16 +55,20 @@ EXPORT_SYMBOL(p9stat_free); >> >> size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size) >> { >> - size_t len = min(pdu->size - pdu->offset, size); >> - memcpy(data, &pdu->sdata[pdu->offset], len); >> + size_t len = pdu->offset > pdu->size ? 0 : >> + min(pdu->size - pdu->offset, size); > I suggest this add two *Tab* lens. > >> + if (len != 0) >> + memcpy(data, &pdu->sdata[pdu->offset], len); >> pdu->offset += len; >> return size - len; >> } >> >> static size_t pdu_write(struct p9_fcall *pdu, const void *data, size_t size) >> { >> - size_t len = min(pdu->capacity - pdu->size, size); >> - memcpy(&pdu->sdata[pdu->size], data, len); >> + size_t len = pdu->size > pdu->capacity ? 0 : >> + min(pdu->capacity - pdu->size, size); > The same as above. > >> + if (len != 0) >> + memcpy(&pdu->sdata[pdu->size], data, len); >> pdu->size += len; >> return size - len; >> } >> >
This patch is not necessary anymore, the one I just sent fixes the length validation issue.
