mirrors/ceph-csi
1
0
Fork 0
mirror of https://github.com/ceph/ceph-csi.git synced 2024-11-30 02:00:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

rbd: prevent calling rbdVolume.Destroy() after an error

Browse Source 
It seems possible that the .Destroy() function is called on a nil
pointer. This would cause a panic in the node-plugin.

Depending on how far GenVolFromVolID() comes, the returned rbdVolume can
be connected. If an error occurs, the rbdVolume should not be connected
at all, so call the .Destroy() function in those cases too.

Fixes: #4562
Signed-off-by: Niels de Vos <ndevos@ibm.com>
This commit is contained in:
Niels de Vos 2024-04-17 12:25:18 +02:00
parent 51d1d46dd2
commit a4b6ef4561
2 changed files with 28 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
internal/rbd/nodeserver.go
View File

@ -198,7 +198,6 @@ func (ns *NodeServer) populateRbdVol(
} else { } else {
rv, err = GenVolFromVolID(ctx, volID, cr, req.GetSecrets()) rv, err = GenVolFromVolID(ctx, volID, cr, req.GetSecrets())
if err != nil { if err != nil {
rv.Destroy()
log.ErrorLog(ctx, "error generating volume %s: %v", volID, err) log.ErrorLog(ctx, "error generating volume %s: %v", volID, err)
return nil, status.Errorf(codes.Internal, "error generating volume %s: %v", volID, err) return nil, status.Errorf(codes.Internal, "error generating volume %s: %v", volID, err)

35
internal/rbd/rbd_util.go
View File

@ -1070,6 +1070,8 @@ func updateSnapshotDetails(rbdSnap *rbdSnapshot) error {
} }
// generateVolumeFromVolumeID generates a rbdVolume structure from the provided identifier. // generateVolumeFromVolumeID generates a rbdVolume structure from the provided identifier.
// The returned rbdVolume is connected in case no error occurred, and should be
// cleaned up with rbdVolume.Destroy().
func generateVolumeFromVolumeID( func generateVolumeFromVolumeID(
ctx context.Context, ctx context.Context,
volumeID string, volumeID string,
@ -1116,6 +1118,13 @@ func generateVolumeFromVolumeID(
if err != nil { if err != nil {
return rbdVol, err return rbdVol, err
} }
// in case of any error call Destroy for cleanup.
defer func() {
if err != nil {
rbdVol.Destroy()
}
}()
rbdVol.JournalPool = rbdVol.Pool rbdVol.JournalPool = rbdVol.Pool
imageAttributes, err := j.GetImageAttributes( imageAttributes, err := j.GetImageAttributes(
@ -1163,6 +1172,8 @@ func generateVolumeFromVolumeID(
// GenVolFromVolID generates a rbdVolume structure from the provided identifier, updating // GenVolFromVolID generates a rbdVolume structure from the provided identifier, updating
// the structure with elements from on-disk image metadata as well. // the structure with elements from on-disk image metadata as well.
// The returned rbdVolume is connected in case no error occurred, and should be
// cleaned up with rbdVolume.Destroy().
func GenVolFromVolID( func GenVolFromVolID(
ctx context.Context, ctx context.Context,
volumeID string, volumeID string,
@ -1185,17 +1196,27 @@ func GenVolFromVolID(
!errors.Is(err, ErrImageNotFound) { !errors.Is(err, ErrImageNotFound) {
return vol, err return vol, err
} }
defer func() {
if err != nil {
vol.Destroy()
}
}()
// Check clusterID mapping exists // Check clusterID mapping exists
mapping, mErr := util.GetClusterMappingInfo(vi.ClusterID) mapping, err := util.GetClusterMappingInfo(vi.ClusterID)
if mErr != nil { if err != nil {
return vol, mErr return vol, err
} }
if mapping != nil { if mapping != nil {
rbdVol, vErr := generateVolumeFromMapping(ctx, mapping, volumeID, vi, cr, secrets) // create a new volume based on the mapping, cleanup the old volume
if !errors.Is(vErr, util.ErrKeyNotFound) && !errors.Is(vErr, util.ErrPoolNotFound) && if vol != nil {
!errors.Is(vErr, ErrImageNotFound) { vol.Destroy()
return rbdVol, vErr }
vol, err = generateVolumeFromMapping(ctx, mapping, volumeID, vi, cr, secrets)
if !errors.Is(err, util.ErrKeyNotFound) && !errors.Is(err, util.ErrPoolNotFound) &&
!errors.Is(err, ErrImageNotFound) {
return vol, err
} }
} }