Skip to contentSkip to conversation

bug list: dereferencing first then checking

PATCH In review Dan Carpenter 3 participants 4 messages Jan 29, 2010 – Feb 7, 2010 1 message not in the archive

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

These bugs are when code dereferences a variable and then checks that it is not null. The new thing is that I wrote a shell script to try remove the false positives caused by macros. There are still some false positives because smatch is bad at handling loops and knowing when a container got redefined.

Sometimes the fixes are not obvious.

This is the output of: /path/to/smatch_scripts/filter_kernel_deref_check.sh warns.txt

regards,
dan carpenter

arch/x86/kernel/apic/io_apic.c +3234 'desc_new': if (desc_new) kernel/trace/trace_events_filter.c +1400 'call': if (!call)
fs/afs/security.c +202 'xpermits': if (xpermits)
fs/btrfs/inode.c +5561 'old_inode': old_inode && S_ISREG(old_inode->i_mode)) { fs/btrfs/free-space-cache.c +569 'info': if (info && info->bitmap) {
fs/cifs/file.c +1035 'file->f_path.dentry': if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) { fs/cifs/file.c +1152 'file->f_path.dentry': if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) { fs/coda/inode.c +204 'vc': if (vc)
fs/configfs/dir.c +1050 'origin': BUG_ON(!origin || !sd);
fs/dlm/lock.c +733 'lkb': if (!lkb)
fs/ecryptfs/crypto.c +347 'crypt_stat': BUG_ON(!crypt_stat || !crypt_stat->tfm fs/efs/inode.c +299 'bh': if (bh) brelse(bh);
fs/efs/inode.c +304 'bh': if (bh) brelse(bh);
fs/ext4/move_extent.c +691 'dext': if (!dext) {
fs/fscache/page.c +712 'page': if (page) {
fs/gfs2/bmap.c +656 'new': BUG_ON(!new);
fs/nfs/nfs4proc.c +4445 'clp': BUG_ON(clp = NULL);
fs/ntfs/attrib.c +350 'ni': BUG_ON(!ni);
fs/ntfs/attrib.c +473 'ni': BUG_ON(!ni);
fs/ntfs/mft.c +1990 'ctx': if (ctx)
fs/ocfs2/aops.c +282 'page': mlog_entry("(0x%p, %lu)\n", file, (page ? page->index : 0)); fs/ocfs2/aops.c +282 'page': mlog_entry("(0x%p, %lu)\n", file, (page ? page->index : 0)); fs/ocfs2/aops.c +282 'page': mlog_entry("(0x%p, %lu)\n", file, (page ? page->index : 0)); fs/ocfs2/dlmglue.c +1524 'inode': BUG_ON(!inode);
fs/ocfs2/dlmglue.c +1572 'inode': BUG_ON(!inode);
fs/ocfs2/dlmglue.c +1625 'inode': BUG_ON(!inode);
fs/ocfs2/dlmglue.c +1653 'inode': BUG_ON(!inode);
fs/ocfs2/dlmglue.c +2252 'inode': BUG_ON(!inode);
fs/ocfs2/journal.c +353 'osb': BUG_ON(!osb || !osb->journal->j_journal);
fs/ocfs2/namei.c +1238 'newfe_bh': mlog(0, "aha rename over existing... new_blkno=%llu " fs/ocfs2/namei.c +1238 'newfe_bh': mlog(0, "aha rename over existing... new_blkno=%llu " fs/ocfs2/namei.c +1238 'newfe_bh': mlog(0, "aha rename over existing... new_blkno=%llu " fs/ocfs2/dlm/dlmrecovery.c +1773 'lock': if (lock)
fs/proc/kcore.c +493 'm': if (m = NULL) {
fs/reiserfs/lbalance.c +895 'bi': if (bi && bi->tb)
fs/reiserfs/ibalance.c +298 'src': RFALSE(dest = NULL || src = NULL,
fs/ubifs/io.c +704 'wbuf': ubifs_assert(wbuf && lnum >= 0 && lnum < c->leb_cnt && offs >= 0); fs/ubifs/tnc.c +1654 'wbuf': ubifs_assert(wbuf && lnum >= 0 && lnum < c->leb_cnt && offs >= 0); fs/ubifs/tnc.c +1900 'znode->parent': if (!znode->parent || znode->iip)
fs/ubifs/find.c +314 'lp': else if (idx_lp && !lp)
fs/xfs/xfs_dir2_leaf.c +1565 'dbp': else if (db != mp->m_dirdatablk && dbp != NULL) { fs/xfs/xfs_dir2_node.c +1140 'args': ASSERT(args != NULL);
fs/xfs/xfs_log_recover.c +143 'bp': ASSERT(bp);
security/smack/smack_lsm.c +2322 'opt_dentry': if (opt_dentry = NULL) { drivers/atm/fore200e.c +1512 'vcc': ASSERT(vcc);
drivers/atm/fore200e.c +1573 'vcc': ASSERT(vcc);
drivers/block/aoe/aoecmd.c +196 '*t': if (t >= &d->targets[NTARGETS] || !*t) drivers/char/ip2/ip2main.c +1637 'tty': if ( !tty || (tty->termios->c_cflag & HUPCL) ) { drivers/char/pcmcia/synclink_cs.c +1100 'tty': if (tty)
drivers/char/pcmcia/synclink_cs.c +1110 'tty': if (tty)
drivers/char/mxser.c +2196 'tty': if (port->xmit_cnt < WAKEUP_CHARS && tty) drivers/char/synclink.c +1372 'info->port.tty': if (info->port.tty) drivers/char/synclink.c +1382 'info->port.tty': if (info->port.tty)

drivers/char/synclink.c +2034 'tty': if (!tty || !info->xmit_buf)
drivers/char/synclink.c +2124 'tty': if (!tty || !info->xmit_buf)

drivers/gpu/drm/nouveau/nouveau_object.c +891 'chan': if (!chan || !gpuobj_ret || *gpuobj_ret != NULL) drivers/gpu/drm/nouveau/nouveau_sgdma.c +61 'nvbe': if (nvbe && nvbe->pages) { drivers/gpu/drm/nouveau/nouveau_connector.c +91 'connector': if (!connector) drivers/gpu/drm/nouveau/nv50_crtc.c +306 'crtc': if (!crtc)
drivers/gpu/drm/radeon/atombios_crtc.c +476 'encoder': if (!encoder) drivers/gpu/drm/radeon/r600_blit.c +619 'dev_priv->blit_vb': if (!dev_priv->blit_vb) drivers/gpu/drm/radeon/r600_blit.c +707 'dev_priv->blit_vb': if (!dev_priv->blit_vb) drivers/gpu/drm/radeon/r600_blit.c +785 'dev_priv->blit_vb': if (!dev_priv->blit_vb) drivers/gpu/drm/drm_lock.c +81 'master->lock.hw_lock': if (!master->lock.hw_lock) { drivers/gpu/drm/drm_memory.c +80 'agpmem': if (!agpmem)
drivers/gpu/drm/drm_vm.c +140 'agpmem': if (!agpmem)
drivers/infiniband/hw/mlx4/cq.c +400 'cq->resize_buf': if (cq->resize_buf) { drivers/isdn/hardware/eicon/message.c +4954 'plci': if(esc_cr[0] && plci)
drivers/isdn/hisax/l3dss1.c +2215 'pc': if (pc) dss1_release_l3_process(pc); drivers/isdn/hisax/l3dss1.c +2220 'pc': if (pc)
drivers/isdn/hisax/l3ni1.c +2071 'pc': if (pc) ni1_release_l3_process(pc);
drivers/isdn/hisax/l3ni1.c +2076 'pc': if (pc)
drivers/isdn/hysdn/hysdn_net.c +193 'lp': if (!lp)
drivers/md/raid5.c +5120 'conf': if (conf) {
drivers/media/dvb/dvb-usb/anysee.c +482 'd': if (d)
drivers/media/dvb/dvb-usb/opera1.c +486 'fw': if (fw) {
drivers/media/dvb/frontends/stv0900_core.c +1357 'state->internal': if (state->internal = NULL) { drivers/media/video/s2255drv.c +2605 'dev': if (dev) {
drivers/media/video/cx18/cx18-dvb.c +256 'stream': if (!stream)
drivers/media/video/cx18/cx18-dvb.c +288 'stream': if (stream) { drivers/media/video/cx231xx/cx231xx-core.c +99 'dev': if (dev) drivers/media/video/cx231xx/cx231xx-core.c +115 'dev': if (dev) drivers/media/video/em28xx/em28xx-core.c +1173 'dev': if (dev) drivers/media/video/em28xx/em28xx-core.c +1189 'dev': if (dev) drivers/media/video/pvrusb2/pvrusb2-ctrl.c +516 'maskptr': if (maskptr) *maskptr = ~0; drivers/media/video/pvrusb2/pvrusb2-ctrl.c +525 'maskptr': if (maskptr) *maskptr = 1; drivers/media/video/pvrusb2/pvrusb2-ctrl.c +534 'maskptr': if (maskptr) *maskptr = ~0; drivers/media/video/pvrusb2/pvrusb2-io.c +476 'sp': if (sp && sp->callback_func) { drivers/media/video/pwc/pwc-ctrl.c +314 'pChoose': if (pChoose = NULL || pChoose->alternate = 0) drivers/media/video/saa7164/saa7164-buffer.c +139 'port': if ((buf = 0) || (port = 0)) drivers/media/video/usbvideo/ibmcam.c +406 'uvd': assert(uvd != NULL); drivers/media/video/usbvideo/quickcam_messenger.c +699 'uvd': if ((uvd = NULL) || (!uvd->streaming) || (uvd->dev = NULL)) drivers/message/fusion/mptbase.c +593 'reply': if (reply) {
drivers/message/i2o/i2o_block.c +724 'dev->i2o_dev': if (unlikely(!dev->i2o_dev)) { drivers/message/i2o/i2o_scsi.c +535 'i2o_dev': if (unlikely(!i2o_dev)) { drivers/mtd/chips/cfi_cmdset_0002.c +497 'mtd': if(mtd) {
drivers/mtd/chips/cfi_cmdset_0001.c +618 'mtd': if(mtd) {
drivers/mtd/devices/m25p80.c +466 'retlen': if (retlen)
drivers/mtd/devices/m25p80.c +784 'plat_id': if (plat_id)
drivers/net/can/usb/ems_usb.c +876 'skb': if (skb)
drivers/net/hamradio/yam.c +871 'dev': if (!dev || !yp->bitrate)
drivers/net/irda/nsc-ircc.c +1262 'self': IRDA_ASSERT(self != NULL, return 0;); drivers/net/pcmcia/xirc2ps_cs.c +1558 'dev': pr_debug("%s: do_reset(%p,%d)\n", dev? dev->name:"eth?", dev, full); drivers/net/qlge/qlge_main.c +1887 'net_rsp': net_rsp != NULL) {
drivers/net/tokenring/tms380tr.c +1355 'fw_entry': if (fw_entry)
drivers/net/tokenring/tms380tr.c +1361 'fw_entry': if (fw_entry)
drivers/net/wireless/at76c50x-usb.c +1533 'urb': if (urb)
drivers/net/wireless/rndis_wlan.c +2815 'priv': if (priv && priv->wpa_ie_len) drivers/net/wireless/mac80211_hwsim.c +482 'data->channel': data->channel || !data2->channel || drivers/net/wireless/b43/main.c +3883 'dev': if (!dev || b43_status(dev) < B43_STAT_STARTED) drivers/net/wireless/ipw2x00/ipw2200.c +8516 'rxb->skb': if (rxb->skb != NULL) { drivers/net/wireless/ipw2x00/libipw_rx.c +803 'skb': if (skb) { drivers/net/wireless/iwmc3200wifi/rx.c +797 'bss': if (!bss) {
drivers/net/wireless/libertas/if_usb.c +361 'priv': if (priv) { drivers/net/wireless/wl12xx/wl1271_main.c +835 'wl': if (wl = NULL)
drivers/net/ppp_generic.c +1395 'pch->chan': if (pch->chan = NULL) {
drivers/net/ppp_synctty.c +598 'skb': if (skb && ap->flags & SC_LOG_OUTPKT) drivers/pcmcia/pcmcia_ioctl.c +749 'link': if (link = NULL)
drivers/scsi/aacraid/aachba.c +1531 'fibptr': BUG_ON(fibptr = NULL);
drivers/scsi/aacraid/commsup.c +1736 'dev->queues': else if (!dev->queues) drivers/scsi/bfa/bfa_fcxp.c +633 'fcxp': bfa_assert(fcxp != NULL);
drivers/scsi/cxgb3i/cxgb3i_pdu.c +464 'c3cn': if (c3cn) {
drivers/scsi/lpfc/lpfc_sli.c +4532 'pmbox': if (pmbox = NULL) {
drivers/scsi/lpfc/lpfc_els.c +2765 'ndlp': if (ndlp && NLP_CHK_NODE_ACT(ndlp) && delay) { drivers/scsi/lpfc/lpfc_scsi.c +2234 'cmd->device': lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP, drivers/scsi/megaraid/megaraid_mm.c +256 'adapter': if (!adapter) { drivers/scsi/megaraid/megaraid_mm.c +754 'adapter': if (adapter) { drivers/scsi/megaraid/megaraid_sas.c +3349 'instance->producer': if (instance->producer) drivers/scsi/megaraid/megaraid_sas.c +3352 'instance->consumer': if (instance->consumer) drivers/scsi/mvsas/mv_sas.c +1363 'mvi_dev': if (mvi_dev) {
drivers/scsi/pm8001/pm8001_sas.c +891 'pm8001_dev': if (pm8001_dev) {
drivers/scsi/qla2xxx/qla_os.c +2125 'vha': if (vha && vha->fc_vport)
drivers/scsi/qla2xxx/qla_init.c +3528 'fcport': if (fcport && fcport->drport && drivers/scsi/ncr53c8xx.c +4799 'np->ccb': if (np->ccb)
drivers/scsi/ncr53c8xx.c +5642 'tp': if ((!tp) || (!lp) || !sdev)
drivers/scsi/ncr53c8xx.c +5642 'sdev': if ((!tp) || (!lp) || !sdev)
drivers/scsi/ips.c +2798 'scb->scsi_cmd': if (scb->scsi_cmd) {
drivers/scsi/ips.c +2810 'scb->scsi_cmd': if (scb->scsi_cmd)
drivers/scsi/ips.c +3292 'scb->scsi_cmd': if (scb->scsi_cmd) {
drivers/scsi/ips.c +3300 'scb->scsi_cmd': if (scb->scsi_cmd) {
drivers/staging/arlan/arlan-proc.c +625 'ctl': if (ctl)
drivers/staging/comedi/drivers/pcmuio.c +471 'dev->private': if (devpriv && devpriv->sprivs) drivers/staging/comedi/drivers/quatech_daqp_cs.c +1084 'dev': if (dev) drivers/staging/cx25821/cx25821-video.c +969 'fh': if (fh) { drivers/staging/cx25821/cx25821-audups11.c +349 'fh': if (fh) {
drivers/staging/hv/storvsc_drv.c +399 'scmnd': ASSERT(scmnd);
drivers/staging/hv/ChannelMgmt.c +644 'msgInfo': if (msgInfo) {
drivers/staging/line6/toneport.c +242 'toneport': if (toneport != NULL) { drivers/staging/pohmelfs/config.c +305 'g': if (g)
drivers/staging/pohmelfs/config.c +315 'g': if (g) {
drivers/staging/pohmelfs/config.c +357 'g': if (g) {
drivers/staging/pohmelfs/dir.c +700 'inode': BUG_ON(!inode); drivers/staging/rt2860/common/rtmp_init.c +3418 'pAd': if (pAd && (Status != NDIS_STATUS_SUCCESS)) { drivers/staging/rt2870/common/../../rt2860/common/rtmp_init.c +3418 'pAd': if (pAd && (Status != NDIS_STATUS_SUCCESS)) { drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c +771 'skb': if (skb) { drivers/staging/rtl8192e/r8192E_core.c +3564 'skb': if(skb){ drivers/staging/rtl8192e/ieee80211/ieee80211_rx.c +598 'sub_skb': if (sub_skb) { drivers/staging/rtl8192e/ieee80211/rtl819x_BAProc.c +117 'ieee': if (pBA = NULL||ieee = NULL) drivers/staging/rtl8192su/r8192U_core.c +1645 'skb': if(skb != NULL) { drivers/staging/rtl8192su/r8192U_core.c +2635 'priv': if (priv = NULL) drivers/staging/rtl8192su/ieee80211/ieee80211_rx.c +546 'sub_skb': if (sub_skb) { drivers/staging/rtl8192su/ieee80211/rtl819x_BAProc.c +117 'ieee': if (pBA = NULL||ieee = NULL) drivers/staging/rtl8192u/r8192U_core.c +1515 'skb': if(skb != NULL) { drivers/staging/rtl8192u/r8192U_core.c +2411 'priv': if (priv = NULL) drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +590 'sub_skb': if (sub_skb) { drivers/staging/rtl8192u/ieee80211/rtl819x_BAProc.c +117 'ieee': if (pBA = NULL||ieee = NULL) drivers/staging/serqt_usb2/serqt_usb2.c +366 'serial': if (!serial) { drivers/staging/slicoss/slicoss.c +492 'adapter': ASSERT(adapter);
drivers/staging/slicoss/slicoss.c +571 'curr_card': ASSERT(curr_card); drivers/staging/slicoss/slicoss.c +2551 'adapter': if ((adapter) && (adapter->state = ADAPT_UP) && drivers/staging/vt6656/main_usb.c +856 'pTxContext': if (pTxContext)
drivers/usb/host/ehci-dbg.c +616 'p.qh': if (p.qh) {
drivers/usb/host/ehci-sched.c +1002 'dev->tt': think_time = dev->tt ? dev->tt->think_time : 0; drivers/usb/serial/io_ti.c +2140 'tty': if (tty)
drivers/usb/gadget/u_serial.c +399 'port->port_usb': if (!port->port_usb) drivers/usb/gadget/u_serial.c +451 'port->port_usb': if (!port->port_usb) drivers/usb/gadget/u_serial.c +399 'port->port_usb': if (!port->port_usb) drivers/usb/gadget/u_serial.c +451 'port->port_usb': if (!port->port_usb) drivers/usb/gadget/u_serial.c +399 'port->port_usb': if (!port->port_usb) drivers/usb/gadget/u_serial.c +451 'port->port_usb': if (!port->port_usb)
sound/pci/ali5451/ali5451.c +880 'pvoice->substream': if (pvoice->pcm && pvoice->substream) { net/9p/trans_rdma.c +247 'c': if (c)
net/dcb/dcbnl.c +1126 'skb': u32 pid = skb ? NETLINK_CB(skb).pid : 0;
net/decnet/dn_route.c +627 'dev': int)flags, (dev) ? dev->name : "???", len, skb->len, net/ipv4/cipso_ipv4.c +480 'doi_def': if (doi_def = NULL || doi_def->doi = CIPSO_V4_DOI_UNKNOWN) net/ipv6/proc.c +262 'idev': if (!idev || !idev->stats.proc_dir_entry)
net/irda/irlap_event.c +1458 'info': IRDA_ASSERT(info != NULL, return -1;);
net/irda/af_irda.c +596 'self->ias_result': if (self->ias_result)
net/irda/ircomm/ircomm_tty.c +499 'tty': if (!tty)
net/irda/ircomm/ircomm_tty.c +1010 'tty': if (!tty)
net/netfilter/nf_conntrack_netlink.c +1486 'exp': NLA_PUT_BE32(skb, CTA_EXPECT_ID, htonl((unsigned long)exp)); net/packet/af_packet.c +818 'skb': BUG_ON(skb = NULL);
net/sched/sch_api.c +348 's': if (!s || tsize != s->tsize || (!tab && tsize > 0))
net/sched/act_api.c +969 'skb': u32 pid = skb ? NETLINK_CB(skb).pid : 0;
net/sched/cls_tcindex.c +343 'arg': pr_debug("tcindex_change(tp %p,handle 0x%08x,tca %p,arg %p),opt %p," net/sctp/sm_statefuns.c +1275 'new_addr': if (!found && new_addr) {
net/sctp/sm_make_chunk.c +472 'chunk': if (chunk)
net/sctp/outqueue.c +332 'chunk': SCTP_DEBUG_PRINTK("outqueueing (%p, %p[%s])\n",
net/sctp/outqueue.c +332 'chunk->chunk_hdr': SCTP_DEBUG_PRINTK("outqueueing (%p, %p[%s])\n", net/sunrpc/xprt.c +204 'task': if (task = NULL)
net/sunrpc/xprt.c +209 'req': if (req) {
net/sunrpc/xprt.c +220 'req': if (req && req->rq_ntrans)
net/sunrpc/cache.c +363 'current_detail': while (current_detail = NULL ||
net/wireless/scan.c +527 'wiphy': if (WARN_ON(!mgmt || !wiphy ||
net/xfrm/xfrm_policy.c +2254 'dst->xfrm': while (dst->xfrm);
lib/radix-tree.c +588 'node': if (node = NULL)

#1 lore raw
Quoted 12 lines · Peter Huewe

Am Freitag 29 Januar 2010 10:00:49 schrieb Dan Carpenter:

Quoted 11 lines

These bugs are when code dereferences a variable and then checks that it is
not null. The new thing is that I wrote a shell script to try remove the false positives caused by macros. There are still some false positives because smatch is bad at handling loops and knowing when a container got redefined.

Sometimes the fixes are not obvious.

This is the output of: /path/to/smatch_scripts/filter_kernel_deref_check.sh
warns.txt

Quoted 8 lines
Quoted 2 lines

regards,
dan carpenter

Hey Dan,

can you please explain to me (on a simple example) what is wrong with these code samples? I somehow don't get it.

The first two in the list are not false positives but let's take number 3.

fs/afs/security.c +202

   169          xpermits = auth_vnode->permits;
   170          count = 0;
   171          if (xpermits) {
   172                  /* see if the permit is already in the list
   173                   * - if it is then we just amend the list
   174                   */
   175                  count = xpermits->count;
   176                  permit = xpermits->permits;
   177                  for (loop = count; loop > 0; loop--) {
   178                          if (permit->key = key) {
   179                                  permit->access_mask    180                                          vnode->status.caller_access;
   181                                  goto out_unlock;
   182                          }
   183                          permit++;
   184                  }
   185          }
   186
   187          permits = kmalloc(sizeof(*permits) + sizeof(*permit) * (count + 1),
   188                            GFP_NOFS);
   189          if (!permits)
   190                  goto out_unlock;
   191
   192          memcpy(permits->permits, xpermits->permits,
   193                 count * sizeof(struct afs_permit));
   194
   195          _debug("key %x access %x",
   196                 key_serial(key), vnode->status.caller_access);
   197          permits->permits[count].access_mask = vnode->status.caller_access;
   198          permits->permits[count].key = key_get(key);
   199          permits->count = count + 1;

200
201 rcu_assign_pointer(auth_vnode->permits, permits);
202 if (xpermits)
203 call_rcu(&xpermits->rcu, afs_dispose_of_permits);

If xpermits were null we would oops on line 192. It does look like xpermits can be null.

I suspect the right fix is to test xpermits on line 170 and goto_unlock if it is null. We could remove the if statements on lines 171 and 202. But it would be better to really dig into it and find out for sure if xpermits can be null.

Quite a few of the fixes are not straightforward. Sometimes maybe a variable could become null inside a function. Smatch wouldn't catch that. Some of the checks could be removed, but if they don't hurt anything is it worth it?

regards,
dan carpenter

#2 lore raw

PATCH 1/1 nouveau: move dereferences after null checks

In reply to Dan Carpenter
Quoted 12 lines · Dan Carpenter

These bugs are when code dereferences a variable and then checks that it is not null. The new thing is that I wrote a shell script to try remove the false positives caused by macros. There are still some false positives because smatch is bad at handling loops and knowing when a container got redefined.

Sometimes the fixes are not obvious.

This is the output of: /path/to/smatch_scripts/filter_kernel_deref_check.sh warns.txt

regards,
dan carpenter

Quoted 6 lines

(...)
drivers/gpu/drm/nouveau/nouveau_object.c +891 'chan': if (!chan || !gpuobj_ret || *gpuobj_ret != NULL) drivers/gpu/drm/nouveau/nouveau_sgdma.c +61 'nvbe': if (nvbe && nvbe->pages) { drivers/gpu/drm/nouveau/nouveau_connector.c +91 'connector': if (!connector) drivers/gpu/drm/nouveau/nv50_crtc.c +306 'crtc': if (!crtc)
(...)

---
From: Marcin Slusarz <[email protected]>
Subject: [PATCH] nouveau: move dereferences after null checks

---
 drivers/gpu/drm/nouveau/nouveau_connector.c |    7 ++++---
 drivers/gpu/drm/nouveau/nouveau_object.c    |    3 ++-
 drivers/gpu/drm/nouveau/nouveau_sgdma.c     |    7 ++++---
 drivers/gpu/drm/nouveau/nv50_crtc.c         |   11 +++++++----
 4 files changed, 17 insertions(+), 11 deletions(-)
drivers/gpu/drm/nouveau/nouveau_connector.c +4 -3
index 7e6d673..d2f6335 100644
@@ -88,13 +88,14 @@ nouveau_connector_destroy(struct drm_connector *drm_connector)
 {
 	struct nouveau_connector *nv_connector  		nouveau_connector(drm_connector);
-	struct drm_device *dev = nv_connector->base.dev;
-
-	NV_DEBUG_KMS(dev, "\n");
+	struct drm_device *dev;
 
 	if (!nv_connector)
 		return;
 
+	dev = nv_connector->base.dev;
+	NV_DEBUG_KMS(dev, "\n");
+
 	kfree(nv_connector->edid);
 	drm_sysfs_connector_remove(drm_connector);
 	drm_connector_cleanup(drm_connector);
drivers/gpu/drm/nouveau/nouveau_object.c +2 -1
index 6c2cf81..e7c100b 100644
@@ -885,11 +885,12 @@ int
 nouveau_gpuobj_sw_new(struct nouveau_channel *chan, int class,
 		      struct nouveau_gpuobj **gpuobj_ret)
 {
-	struct drm_nouveau_private *dev_priv = chan->dev->dev_private;
+	struct drm_nouveau_private *dev_priv;
 	struct nouveau_gpuobj *gpuobj;
 
 	if (!chan || !gpuobj_ret || *gpuobj_ret != NULL)
 		return -EINVAL;
+	dev_priv = chan->dev->dev_private;
 
 	gpuobj = kzalloc(sizeof(*gpuobj), GFP_KERNEL);
 	if (!gpuobj)
drivers/gpu/drm/nouveau/nouveau_sgdma.c +4 -3
index 4c7f1e4..ed15905 100644
@@ -54,11 +54,12 @@ static void
 nouveau_sgdma_clear(struct ttm_backend *be)
 {
 	struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)be;
-	struct drm_device *dev = nvbe->dev;
-
-	NV_DEBUG(nvbe->dev, "\n");
+	struct drm_device *dev;
 
 	if (nvbe && nvbe->pages) {
+		dev = nvbe->dev;
+		NV_DEBUG(dev, "\n");
+
 		if (nvbe->bound)
 			be->func->unbind(be);
 
drivers/gpu/drm/nouveau/nv50_crtc.c +7 -4
index 40b7360..d1a651e 100644
@@ -298,14 +298,17 @@ nv50_crtc_set_clock(struct drm_device *dev, int head, int pclk)
 static void
 nv50_crtc_destroy(struct drm_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
-	NV_DEBUG_KMS(dev, "\n");
+	struct drm_device *dev;
+	struct nouveau_crtc *nv_crtc;
 
 	if (!crtc)
 		return;
 
+	dev = crtc->dev;
+	nv_crtc = nouveau_crtc(crtc);
+
+	NV_DEBUG_KMS(dev, "\n");
+
 	drm_crtc_cleanup(&nv_crtc->base);
 
 	nv50_cursor_fini(nv_crtc);
Signature · 1 line
1.6.6

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

Re: [PATCH] nouveau: move dereferences after null checks

Quoted 20 lines · Marcin Slusarz
Quoted 12 lines · Dan Carpenter

These bugs are when code dereferences a variable and then checks that it is not null. The new thing is that I wrote a shell script to try remove the false positives caused by macros. There are still some false positives because smatch is bad at handling loops and knowing when a container got redefined.

Sometimes the fixes are not obvious.

This is the output of: /path/to/smatch_scripts/filter_kernel_deref_check.sh warns.txt

regards,
dan carpenter

Quoted 6 lines

(...)
drivers/gpu/drm/nouveau/nouveau_object.c +891 'chan': if (!chan || !gpuobj_ret || *gpuobj_ret != NULL) drivers/gpu/drm/nouveau/nouveau_sgdma.c +61 'nvbe': if (nvbe && nvbe->pages) { drivers/gpu/drm/nouveau/nouveau_connector.c +91 'connector': if (!connector) drivers/gpu/drm/nouveau/nv50_crtc.c +306 'crtc': if (!crtc)
(...)

Quoted 7 lines

---
From: Marcin Slusarz <[email protected]>
Subject: [PATCH] nouveau: move dereferences after null checks

---

ping