re: scsi: sg: Replace sg_allow_access()

From: Colin Ian King
Date: Thu Mar 11 2021 - 06:08:51 EST


Hi,

Static analysis on linux-next with Coverity has detected an issue in
drivers/scsi/sg.c in function sg_remove_sfp_usercontext with the
following recent commit:

commit 0c32296d73ec5dec64729eb555f1a29ded8a7272
Author: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
Date: Fri Feb 19 21:00:28 2021 -0500

scsi: sg: Replace sg_allow_access()

The analysis is as follows:

3913 if (unlikely(sfp != e_sfp))
3914 SG_LOG(1, sfp, "%s: xa_erase() return unexpected\n",
3915 __func__);

deref_ptr_in_call: Dereferencing pointer sdp.

3916 o_count = atomic_dec_return(&sdp->open_cnt);
3917 SG_LOG(3, sfp, "%s: dev o_count after=%d: sfp=0x%pK --\n",
__func__,
3918 o_count, sfp);
3919 kfree(sfp);
3920

Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking sdp suggests that it may be null,
but it has already been dereferenced on all paths leading to the check.

3921 if (sdp) {
3922 scsi_device_put(sdp->device);
3923 kref_put(&sdp->d_ref, sg_device_destroy);
3924 }

Line 3916 dereferences pointer sdp with &sdp->open_cnt, however later on
in line 3921 sdp is being null checked. Either the null check is
redundant if sdp is never null or there is a potential null pointer
dereference on line 3916.

Colin