Skip to content

Commit 72a4c50

Browse files
[vioscsi] Limit maximum LUN index in TransportReset() and ParamChange()
The 'Lun' variable in TransportReset() and ParamChange() is rightly declared as a UCHAR value. However, the value is derived from two UCHAR members of a 'VirtIOSCSIEvent' struct ('lun[2]' and 'lun[3]'). When the 'lun[2]' member is left shifted 8 bits it is promoted to type USHORT, and then bitwise OR'ed with 'lun[3]'. This leads to an integer truncation if the LUN index is greater than 256. Whilst Storport does not support more than 255 LUNs per Target as defined by SCSI_MAXIMUM_LUNS_PER_TARGET, a malicious or misconfigured hypervisor could send an event for a higher LUN index. In TransportReset() this could result in another LUN being reset, and in ParamChange() this could result in processing parameter changes for the wrong LUN. The solution in this commit is to first obtain the LUN index via new USHORT variable 'lun_candidate' and then check if it is within the bounds of SCSI_MAXIMUM_LUNS_PER_TARGET. If out-of-bounds, we log a warning message - including the reason for the event - and return without further processing. Otherwise we cast 'lun_candidate` to 'Lun' as a UCHAR and continue. Signed-off-by: benyamin-codez <115509179+benyamin-codez@users.noreply.github.com>
1 parent 17afb4d commit 72a4c50

1 file changed

Lines changed: 28 additions & 2 deletions

File tree

vioscsi/vioscsi.c

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,7 +1783,20 @@ VOID LogError(IN PVOID DeviceExtension, IN ULONG ErrorCode, IN ULONG UniqueId)
17831783
VOID TransportReset(IN PVOID DeviceExtension, IN PVirtIOSCSIEvent evt)
17841784
{
17851785
UCHAR TargetId = evt->lun[1];
1786-
UCHAR Lun = (evt->lun[2] << 8) | evt->lun[3];
1786+
USHORT lun_candidate = (evt->lun[2] << 8) | evt->lun[3];
1787+
if (lun_candidate > SCSI_MAXIMUM_LUNS_PER_TARGET - 1)
1788+
{
1789+
RhelDbgPrint(TRACE_LEVEL_WARNING,
1790+
" The LUN specified (%d) is out-of-bounds."
1791+
" Storport can only address a maximum of %d LUNs."
1792+
" The event reason was 0x%x."
1793+
" Returning...\n",
1794+
lun_candidate,
1795+
SCSI_MAXIMUM_LUNS_PER_TARGET,
1796+
evt->reason);
1797+
return;
1798+
}
1799+
UCHAR Lun = (UCHAR)lun_candidate;
17871800
ENTER_FN();
17881801

17891802
switch (evt->reason)
@@ -1803,7 +1816,20 @@ VOID TransportReset(IN PVOID DeviceExtension, IN PVirtIOSCSIEvent evt)
18031816
VOID ParamChange(IN PVOID DeviceExtension, IN PVirtIOSCSIEvent evt)
18041817
{
18051818
UCHAR TargetId = evt->lun[1];
1806-
UCHAR Lun = (evt->lun[2] << 8) | evt->lun[3];
1819+
USHORT lun_candidate = (evt->lun[2] << 8) | evt->lun[3];
1820+
if (lun_candidate > SCSI_MAXIMUM_LUNS_PER_TARGET - 1)
1821+
{
1822+
RhelDbgPrint(TRACE_LEVEL_WARNING,
1823+
" The LUN specified (%d) is out-of-bounds."
1824+
" Storport can only address a maximum of %d LUNs."
1825+
" The event reason was 0x%x."
1826+
" Returning...\n",
1827+
lun_candidate,
1828+
SCSI_MAXIMUM_LUNS_PER_TARGET,
1829+
evt->reason);
1830+
return;
1831+
}
1832+
UCHAR Lun = (UCHAR)lun_candidate;
18071833
UCHAR AdditionalSenseCode = (UCHAR)(evt->reason & 255);
18081834
UCHAR AdditionalSenseCodeQualifier = (UCHAR)(evt->reason >> 8);
18091835
ENTER_FN();

0 commit comments

Comments
 (0)