re: scsi: sg: Replace sg_allow_access()

* re: scsi: sg: Replace sg_allow_access()
@ 2021-03-11 11:07 Colin Ian King
  0 siblings, 0 replies; only message in thread
From: Colin Ian King @ 2021-03-11 11:07 UTC (permalink / raw)
  To: Doug Gilbert
  Cc: James E.J. Bottomley, James E.J. Bottomley, linux-scsi, linux-kernel

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@interlog.com>
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

^ permalink raw reply	[flat|nested] only message in thread