Fix nil pointer dereference in addSCSI #1497
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new change that we added to fix a race condition in addSCSI introduced a bug where the code ends up accessing a nil pointer in certain situations. For example, the deferred function to unblock any waiters of the attach SCSI operation accesses the scsi mount object to propagate any errors. However, this pointer is a named return value of the function and is set to `nil` when returning an error. In those cases the deferred function panics with the nil pointer dereference error. To fix this we don't use the named return value for the scsi mount object anymore. This change also removes the check for zero SCSI controllers since that check is done by the `allocateSCSIMount` function. Signed-off-by: Amit Barve <ambarve@microsoft.com>
helsaawy
approved these changes
Sep 1, 2022
KenGordon
pushed a commit
to KenGordon/hcsshim
that referenced
this pull request
May 17, 2024
The new change that we added to fix a race condition in addSCSI introduced a bug where the code ends up accessing a nil pointer in certain situations. For example, the deferred function to unblock any waiters of the attach SCSI operation accesses the scsi mount object to propagate any errors. However, this pointer is a named return value of the function and is set to `nil` when returning an error. In those cases the deferred function panics with the nil pointer dereference error. To fix this we don't use the named return value for the scsi mount object anymore. This change also removes the check for zero SCSI controllers since that check is done by the `allocateSCSIMount` function. Signed-off-by: Amit Barve <ambarve@microsoft.com> Signed-off-by: Amit Barve <ambarve@microsoft.com> (cherry picked from commit 895853a) Signed-off-by: Amit Barve <ambarve@microsoft.com>
KenGordon
pushed a commit
to KenGordon/hcsshim
that referenced
this pull request
May 17, 2024
…1497) Fix nil pointer dereference in addSCSI (microsoft#1497) The new change that we added to fix a race condition in addSCSI introduced a bug where the code ends up accessing a nil pointer in certain situations. For example, the deferred function to unblock any waiters of the attach SCSI operation accesses the scsi mount object to propagate any errors. However, this pointer is a named return value of the function and is set to `nil` when returning an error. In those cases the deferred function panics with the nil pointer dereference error. To fix this we don't use the named return value for the scsi mount object anymore. This change also removes the check for zero SCSI controllers since that check is done by the `allocateSCSIMount` function. Signed-off-by: Amit Barve <ambarve@microsoft.com> Signed-off-by: Amit Barve <ambarve@microsoft.com> (cherry picked from commit 895853a) Signed-off-by: Amit Barve <ambarve@microsoft.com> Related work items: microsoft#1497
princepereira
pushed a commit
to princepereira/hcsshim
that referenced
this pull request
Aug 29, 2024
The new change that we added to fix a race condition in addSCSI introduced a bug where the code ends up accessing a nil pointer in certain situations. For example, the deferred function to unblock any waiters of the attach SCSI operation accesses the scsi mount object to propagate any errors. However, this pointer is a named return value of the function and is set to `nil` when returning an error. In those cases the deferred function panics with the nil pointer dereference error. To fix this we don't use the named return value for the scsi mount object anymore. This change also removes the check for zero SCSI controllers since that check is done by the `allocateSCSIMount` function. Signed-off-by: Amit Barve <ambarve@microsoft.com> Signed-off-by: Amit Barve <ambarve@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new change that we added to fix a race condition in addSCSI introduced a bug where the code ends up accessing a
nil pointer in certain situations. For example, the deferred function to unblock any waiters of the attach
SCSI operation accesses the scsi mount object to propagate any errors. However, this pointer is a named return
value of the function and is set to
nilwhen returning an error. In those cases the deferred function panicswith the nil pointer dereference error. To fix this we don't use the named return value for the scsi mount
object anymore.
This change also removes the check for zero SCSI controllers since that check is done by the
allocateSCSIMountfunction.Signed-off-by: Amit Barve ambarve@microsoft.com