Skip to contentSkip to conversation

mmc: bcm2835: Add new driver for the sdhost controller.

bug report Dan Carpenter 3 participants 3 messages Mar 21, 2017 – Mar 24, 2017 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

Hello Eric Anholt,

This is a semi-automatic email about new static checker warnings.

The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost controller." from Mar 8, 2017, leads to the following Smatch complaint:

drivers/mmc/host/bcm2835.c:1203 bcm2835_request()

	 error: we previously assumed 'host->mrq->data' could be null (see line 1186)

drivers/mmc/host/bcm2835.c
1156 static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq) 1157 {

  1158          struct bcm2835_host *host = mmc_priv(mmc);
  1159          struct device *dev = &host->pdev->dev;
  1160          u32 edm, fsm;
  1161  
  1162          /* Reset the error statuses in case this is a retry */
  1163          if (mrq->sbc)
  1164                  mrq->sbc->error = 0;
  1165          if (mrq->cmd)
  1166                  mrq->cmd->error = 0;
  1167          if (mrq->data)
  1168                  mrq->data->error = 0;
  1169          if (mrq->stop)
  1170                  mrq->stop->error = 0;
  1171  
  1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
                    ^^^^^^^^^

Tested here.

1173 dev_err(dev, "unsupported block size (%d bytes)\n",
1174 mrq->data->blksz);

  1175                  mrq->cmd->error = -EINVAL;
  1176                  mmc_request_done(mmc, mrq);
  1177                  return;
  1178          }
  1179  
  1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
                                     ^^^^^^^^^

And here.

  1181                  bcm2835_prepare_dma(host, mrq->data);
  1182  
  1183          mutex_lock(&host->mutex);
  1184  
  1185		WARN_ON(host->mrq);
  1186		host->mrq = mrq;

Now host->mrq->data is possibly NULL.

1187
1188 edm = readl(host->ioaddr + SDEDM);
1189 fsm = edm & SDEDM_FSM_MASK;
1190
1191 if ((fsm != SDEDM_FSM_IDENTMODE) &&
1192 (fsm != SDEDM_FSM_DATAMODE)) {
1193 dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
1194 readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
1195 edm);
1196 bcm2835_dumpregs(host);
1197 mrq->cmd->error = -EILSEQ;
1198 bcm2835_finish_request(host);
1199 mutex_unlock(&host->mutex);
1200 return;
1201 }
1202

  1203		host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
                                               ^^^^^^^^^^^^^^^^^

Dereferenced without a test.

  1204		if (host->use_sbc) {
  1205			if (bcm2835_send_command(host, mrq->sbc)) {

regards,
dan carpenter

#1 lore raw
Quoted 68 lines · Dan Carpenter

Hello Eric Anholt,

This is a semi-automatic email about new static checker warnings.

The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost controller." from Mar 8, 2017, leads to the following Smatch complaint:

drivers/mmc/host/bcm2835.c:1203 bcm2835_request()

	 error: we previously assumed 'host->mrq->data' could be null (see line 1186)

drivers/mmc/host/bcm2835.c
1156 static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq) 1157 {

  1158          struct bcm2835_host *host = mmc_priv(mmc);
  1159          struct device *dev = &host->pdev->dev;
  1160          u32 edm, fsm;
  1161  
  1162          /* Reset the error statuses in case this is a retry */
  1163          if (mrq->sbc)
  1164                  mrq->sbc->error = 0;
  1165          if (mrq->cmd)
  1166                  mrq->cmd->error = 0;
  1167          if (mrq->data)
  1168                  mrq->data->error = 0;
  1169          if (mrq->stop)
  1170                  mrq->stop->error = 0;
  1171  
  1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
                    ^^^^^^^^^

Tested here.

1173 dev_err(dev, "unsupported block size (%d bytes)\n",
1174 mrq->data->blksz);

  1175                  mrq->cmd->error = -EINVAL;
  1176                  mmc_request_done(mmc, mrq);
  1177                  return;
  1178          }
  1179  
  1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
                                     ^^^^^^^^^

And here.

  1181                  bcm2835_prepare_dma(host, mrq->data);
  1182  
  1183          mutex_lock(&host->mutex);
  1184  
  1185		WARN_ON(host->mrq);
  1186		host->mrq = mrq;

Now host->mrq->data is possibly NULL.

1187
1188 edm = readl(host->ioaddr + SDEDM);
1189 fsm = edm & SDEDM_FSM_MASK;
1190
1191 if ((fsm != SDEDM_FSM_IDENTMODE) &&
1192 (fsm != SDEDM_FSM_DATAMODE)) {
1193 dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
1194 readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
1195 edm);
1196 bcm2835_dumpregs(host);
1197 mrq->cmd->error = -EILSEQ;
1198 bcm2835_finish_request(host);
1199 mutex_unlock(&host->mutex);
1200 return;
1201 }
1202

Quoted 3 lines
  1203		host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
                                               ^^^^^^^^^^^^^^^^^

Dereferenced without a test.

I think that it can be just added the condition checking for host->mrq->data.

Best Regards,
Jaehoon Chung

Quoted 12 lines
  1204		if (host->use_sbc) {
  1205			if (bcm2835_send_command(host, mrq->sbc)) {

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to Dan Carpenter

+Gerd

Quoted 75 lines · Dan Carpenter

Hello Eric Anholt,

This is a semi-automatic email about new static checker warnings.

The patch 5aa25d971ab4: "mmc: bcm2835: Add new driver for the sdhost controller." from Mar 8, 2017, leads to the following Smatch complaint:

drivers/mmc/host/bcm2835.c:1203 bcm2835_request()

         error: we previously assumed 'host->mrq->data' could be null (see line 1186)

drivers/mmc/host/bcm2835.c
1156 static void bcm2835_request(struct mmc_host *mmc, struct mmc_request *mrq) 1157 {

  1158          struct bcm2835_host *host = mmc_priv(mmc);
  1159          struct device *dev = &host->pdev->dev;
  1160          u32 edm, fsm;
  1161
  1162          /* Reset the error statuses in case this is a retry */
  1163          if (mrq->sbc)
  1164                  mrq->sbc->error = 0;
  1165          if (mrq->cmd)
  1166                  mrq->cmd->error = 0;
  1167          if (mrq->data)
  1168                  mrq->data->error = 0;
  1169          if (mrq->stop)
  1170                  mrq->stop->error = 0;
  1171
  1172          if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
                    ^^^^^^^^^

Tested here.

1173 dev_err(dev, "unsupported block size (%d bytes)\n",
1174 mrq->data->blksz);

  1175                  mrq->cmd->error = -EINVAL;
  1176                  mmc_request_done(mmc, mrq);
  1177                  return;
  1178          }
  1179
  1180          if (host->use_dma && mrq->data && (mrq->data->blocks > PIO_THRESHOLD))
                                     ^^^^^^^^^

And here.

  1181                  bcm2835_prepare_dma(host, mrq->data);
  1182
  1183          mutex_lock(&host->mutex);
  1184
  1185          WARN_ON(host->mrq);
  1186          host->mrq = mrq;

Now host->mrq->data is possibly NULL.

1187
1188 edm = readl(host->ioaddr + SDEDM);
1189 fsm = edm & SDEDM_FSM_MASK;
1190
1191 if ((fsm != SDEDM_FSM_IDENTMODE) &&
1192 (fsm != SDEDM_FSM_DATAMODE)) {
1193 dev_err(dev, "previous command (%d) not complete (EDM %08x)\n",
1194 readl(host->ioaddr + SDCMD) & SDCMD_CMD_MASK,
1195 edm);

  1196                  bcm2835_dumpregs(host);
  1197                  mrq->cmd->error = -EILSEQ;
  1198                  bcm2835_finish_request(host);
  1199                  mutex_unlock(&host->mutex);
  1200                  return;
  1201          }
  1202
  1203          host->use_sbc = !!mrq->sbc && (host->mrq->data->flags & MMC_DATA_READ);
                                               ^^^^^^^^^^^^^^^^^

Dereferenced without a test.

  1204          if (host->use_sbc) {
  1205                  if (bcm2835_send_command(host, mrq->sbc)) {
Quoted 6 lines

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gerd, Eric - can you please look into posting a fix for this!?

Kind regards
Uffe