On 09/12/2022 02:56, Marek Vasut wrote: > On 12/5/22 10:28, Szymon Heidrich wrote: >> Prevent access to arbitrary memory locations in gen_ndis_set_resp >> via manipulation of buf->InformationBufferOffset. Original >> implementation permits manipulation of InformationBufferOffset to >> exploit OID_GEN_CURRENT_PACKET_FILTER to set arbitrary memory contents >> within a 32byte offset as the devices packet filter. The packet filter >> value may be next retrieved using gen_ndis_query_resp so it is possible >> to extract specific memory regions two bytes a time. >> >> The rndis_query_response was not modified as neither the buffer offset >> nor length passed to gen_ndis_query_resp is used. >> >> Signed-off-by: Szymon Heidrich <szymon.heidr...@gmail.com> >> --- >> V1 -> V2: Updated commit message >> >> drivers/usb/gadget/rndis.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c >> index 13c327ea38..3948f2cc9a 100644 >> --- a/drivers/usb/gadget/rndis.c >> +++ b/drivers/usb/gadget/rndis.c >> @@ -855,14 +855,17 @@ static int rndis_set_response(int configNr, >> rndis_set_msg_type *buf) >> rndis_set_cmplt_type *resp; >> rndis_resp_t *r; >> + BufLength = get_unaligned_le32(&buf->InformationBufferLength); >> + BufOffset = get_unaligned_le32(&buf->InformationBufferOffset); >> + if ((BufOffset > RNDIS_MAX_TOTAL_SIZE - 8) || >> + (BufLength > RNDIS_MAX_TOTAL_SIZE - 8 - BufOffset)) >> + return -EINVAL; >> + >> r = rndis_add_response(configNr, sizeof(rndis_set_cmplt_type)); >> if (!r) >> return -ENOMEM; >> resp = (rndis_set_cmplt_type *) r->buf; >> - BufLength = get_unaligned_le32(&buf->InformationBufferLength); >> - BufOffset = get_unaligned_le32(&buf->InformationBufferOffset); >> - >> #ifdef VERBOSE >> debug("%s: Length: %d\n", __func__, BufLength); >> debug("%s: Offset: %d\n", __func__, BufOffset); > > Applied to usb/master, thanks
Thank you very much for your time and review.