All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: blogic@openwrt.org
Cc: devel@driverdev.osuosl.org, NeilBrown <neil@brown.name>,
	linux-mediatek@lists.infradead.org
Subject: [bug report] staging: mt7621-mmc: MIPS: ralink: add sdhci for mt7620a SoC
Date: Thu, 5 Apr 2018 15:13:10 +0300	[thread overview]
Message-ID: <20180405121310.GA7782@mwanda> (raw)

[ I just decided to forward you guys all the Smatch warnings.  -dan ]

Hello John Crispin,

The patch 8b634a9c7620: "staging: mt7621-mmc: MIPS: ralink: add sdhci
for mt7620a SoC" from Mar 15, 2018, leads to the following static
checker warning:

	drivers/staging/mt7621-mmc/sd.c:951 msdc_command_start()
	warn: we tested 'opcode == 3' before and it was 'false'

drivers/staging/mt7621-mmc/sd.c
   931  static unsigned int msdc_command_start(struct msdc_host   *host, 
   932                                        struct mmc_command *cmd,
   933                                        int                 tune,   /* not used */
   934                                        unsigned long       timeout)
   935  {
   936      u32 base = host->base;
   937      u32 opcode = cmd->opcode;
   938      u32 rawcmd;
   939      u32 wints = MSDC_INT_CMDRDY  | MSDC_INT_RSPCRCERR  | MSDC_INT_CMDTMO  |  
   940                  MSDC_INT_ACMDRDY | MSDC_INT_ACMDCRCERR | MSDC_INT_ACMDTMO | 
   941                  MSDC_INT_ACMD19_DONE;  
   942                     
   943      u32 resp;  
   944      unsigned long tmo;
   945  
   946      /* Protocol layer does not provide response type, but our hardware needs 
   947       * to know exact type, not just size!
   948       */
   949      if (opcode == MMC_SEND_OP_COND || opcode == SD_APP_OP_COND)
   950          resp = RESP_R3;
   951      else if (opcode == MMC_SET_RELATIVE_ADDR || opcode == SD_SEND_RELATIVE_ADDR)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
MMC_SET_RELATIVE_ADDR and SD_SEND_RELATIVE_ADDR are both 3 so this
is redundant.

   952          resp = (mmc_cmd_type(cmd) == MMC_CMD_BCR) ? RESP_R6 : RESP_R1;
   953      else if (opcode == MMC_FAST_IO)
   954          resp = RESP_R4;
   955      else if (opcode == MMC_GO_IRQ_STATE)
   956          resp = RESP_R5;
   957      else if (opcode == MMC_SELECT_CARD)
   958          resp = (cmd->arg != 0) ? RESP_R1B : RESP_NONE;
   959      else if (opcode == SD_IO_RW_DIRECT || opcode == SD_IO_RW_EXTENDED)
   960          resp = RESP_R1; /* SDIO workaround. */
   961      else if (opcode == SD_SEND_IF_COND && (mmc_cmd_type(cmd) == MMC_CMD_BCR))
   962          resp = RESP_R1;
   963      else {
   964          switch (mmc_resp_type(cmd)) {

        drivers/staging/mt7621-mmc/sd.c:2961 msdc_drv_suspend()
        warn: variable dereferenced before check 'mmc' (see line 2959)

        drivers/staging/mt7621-mmc/sd.c:2976 msdc_drv_resume()
        warn: variable dereferenced before check 'mmc' (see line 2972)

drivers/staging/mt7621-mmc/sd.c
  2953  /* Fix me: Power Flow */
  2954  #ifdef CONFIG_PM
  2955  static int msdc_drv_suspend(struct platform_device *pdev, pm_message_t state)
  2956  {
  2957      int ret = 0;
  2958      struct mmc_host *mmc = platform_get_drvdata(pdev);
  2959      struct msdc_host *host = mmc_priv(mmc);
                                     ^^^^^^^^^^^^^
Dereference

  2960  
  2961      if (mmc && state.event == PM_EVENT_SUSPEND && (host->hw->flags & MSDC_SYS_SUSPEND)) { /* will set for card */
                ^^^
Check

  2962          msdc_pm(state, (void*)host);
  2963      }
  2964      
  2965      return ret;
  2966  }
  2967
  2968  static int msdc_drv_resume(struct platform_device *pdev)
  2969  {
  2970      int ret = 0;
  2971      struct mmc_host *mmc = platform_get_drvdata(pdev);
  2972      struct msdc_host *host = mmc_priv(mmc);
                                     ^^^^^^^^^^^^
Dereference

  2973      struct pm_message state;
  2974  
  2975      state.event = PM_EVENT_RESUME;
  2976      if (mmc && (host->hw->flags & MSDC_SYS_SUSPEND)) {/* will set for card */
                ^^^
Check

  2977          msdc_pm(state, (void*)host);
  2978      }
  2979  
  2980      /* This mean WIFI not controller by PM */
  2981      
  2982      return ret;
  2983  }

        drivers/staging/mt7621-mmc/dbg.c:270 msdc_debug_proc_write()
        warn: copy_to/from_user() returns a positive value

drivers/staging/mt7621-mmc/dbg.c
   257  static ssize_t msdc_debug_proc_write(struct file *file, 
   258                          const char __user *buf, size_t count, loff_t *data)
   259  {
   260          int ret;
   261          
   262          int cmd, p1, p2;   
   263          int id, zone;
   264          int mode, size;  
   265    
   266          if (count == 0)return -1;
   267          if(count > 255)count = 255;
   268  
   269          ret = copy_from_user(cmd_buf, buf, count);
   270          if (ret < 0)return -1;

This should be:

		if (copy_from_user(cmd_buf, buf, count))
			return -EFAULT;

   271          
   272          cmd_buf[count] = '\0';
   273          printk("msdc Write %s\n", cmd_buf);
   274  

        drivers/staging/mt7621-mmc/dbg.c:339 msdc_debug_proc_init()
        warn: proc file '"msdc_debug"' is world writable

	drivers/staging/mt7621-mmc/dbg.c:341 msdc_debug_proc_init()
	warn: 'de' isn't an ERR_PTR


drivers/staging/mt7621-mmc/dbg.c
   337  int msdc_debug_proc_init(void) 
   338  {       
   339      struct proc_dir_entry *de = proc_create("msdc_debug", 0667, NULL, &msdc_debug_fops);
                                                                     ^
This should probably be a zero instead of a seven.

   340  
   341      if (!de || IS_ERR(de))
                       ^^^^^^^^^^
Remove this.

   342          printk("!! Create MSDC debug PROC fail !!\n");
   343      
   344      return 0 ;
   345  }

regards,
dan carpenter

             reply	other threads:[~2018-04-05 12:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-05 12:13 Dan Carpenter [this message]
2018-04-05 12:26 ` [bug report] staging: mt7621-mmc: MIPS: ralink: add sdhci for mt7620a SoC John Crispin
2018-04-05 16:39 ` Christian Lütke-Stetzkamp
2018-04-06  7:09   ` Dan Carpenter
2018-04-06  7:39     ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2018-04-05 11:56 Dan Carpenter
2018-04-05 12:26 ` John Crispin
2018-04-05 16:46 ` Christian Lütke-Stetzkamp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180405121310.GA7782@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=blogic@openwrt.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=neil@brown.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.