* bug list: dereferencing first then checking
@ 2010-01-29 9:00 Dan Carpenter
2010-01-29 9:54 ` Dan Carpenter
2010-01-30 14:41 ` [PATCH] nouveau: move dereferences after null checks Marcin Slusarz
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2010-01-29 9:00 UTC (permalink / raw)
To: kernel-janitors; +Cc: linux-kernel
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)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: bug list: dereferencing first then checking
2010-01-29 9:00 bug list: dereferencing first then checking Dan Carpenter
@ 2010-01-29 9:54 ` Dan Carpenter
2010-01-30 14:41 ` [PATCH] nouveau: move dereferences after null checks Marcin Slusarz
1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2010-01-29 9:54 UTC (permalink / raw)
To: kernel-janitors
On Fri, Jan 29, 2010 at 10:23:42AM +0100, Peter Huewe wrote:
> Am Freitag 29 Januar 2010 10:00:49 schrieb 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
>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] nouveau: move dereferences after null checks
2010-01-29 9:00 bug list: dereferencing first then checking Dan Carpenter
2010-01-29 9:54 ` Dan Carpenter
@ 2010-01-30 14:41 ` Marcin Slusarz
1 sibling, 0 replies; 3+ messages in thread
From: Marcin Slusarz @ 2010-01-30 14:41 UTC (permalink / raw)
To: nouveau; +Cc: Dan Carpenter, kernel-janitors, linux-kernel
On Fri, Jan 29, 2010 at 12:00:49PM +0300, Dan Carpenter wrote:
> 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
>
> (...)
> 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 <marcin.slusarz@gmail.com>
Subject: [PATCH] nouveau: move dereferences after null checks
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
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(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7e6d673..d2f6335 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -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);
diff --git a/drivers/gpu/drm/nouveau/nouveau_object.c b/drivers/gpu/drm/nouveau/nouveau_object.c
index 6c2cf81..e7c100b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_object.c
+++ b/drivers/gpu/drm/nouveau/nouveau_object.c
@@ -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)
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 4c7f1e4..ed15905 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -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);
diff --git a/drivers/gpu/drm/nouveau/nv50_crtc.c b/drivers/gpu/drm/nouveau/nv50_crtc.c
index 40b7360..d1a651e 100644
--- a/drivers/gpu/drm/nouveau/nv50_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv50_crtc.c
@@ -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);
--
1.6.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-01-30 14:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-29 9:00 bug list: dereferencing first then checking Dan Carpenter
2010-01-29 9:54 ` Dan Carpenter
2010-01-30 14:41 ` [PATCH] nouveau: move dereferences after null checks Marcin Slusarz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).