All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: draviv@codeaurora.org
Cc: linux-scsi@vger.kernel.org
Subject: re: ufs: add ioctl interface for query request
Date: Thu, 9 Apr 2015 23:17:55 +0300	[thread overview]
Message-ID: <20150409201755.GA9651@mwanda> (raw)

Hello Dolev Raviv,

The patch 4aca8e8975db: "ufs: add ioctl interface for query request"
from Mar 12, 2015, leads to the following Smatch warning:

	drivers/scsi/ufs/ufshcd.c:4386 ufshcd_query_ioctl()
	warn: maybe return -EFAULT instead of the bytes remaining?

drivers/scsi/ufs/ufshcd.c
  4364          /* copy to user */
  4365          err = copy_to_user(buffer, ioctl_data,
  4366                          sizeof(struct ufs_ioctl_query_data));
  4367          if (err)
  4368                  dev_err(hba->dev, "%s: Failed copying back to user.\n",
  4369                          __func__);

copy_to/from_user() returns the number of bytes not copied and not an
error code.  Printing these error messages in the ioctl means the user
can trigger a DoS by filling up /var/log/messages.  They make the code
uglier.  We should stop here if the copy fails and goto out_release_mem
otherwise we might end up returning success by mistake.

The normal way to do it is:

	if (copy_to_user(buffer, ioctl_data,
			 sizeof(struct ufs_ioctl_query_data))) {
		err = -EFAULT;
		goto out_release_mem;
	}

  4370          err = copy_to_user(buffer + sizeof(struct ufs_ioctl_query_data),
  4371                          data_ptr, ioctl_data->buf_size);
  4372          if (err)
  4373                  dev_err(hba->dev, "%s: err %d copying back to user.\n",
  4374                                  __func__, err);
  4375          goto out_release_mem;
  4376  
  4377  out_einval:
  4378          dev_err(hba->dev,
  4379                  "%s: illegal ufs query ioctl data, opcode 0x%x, idn 0x%x\n",
  4380                  __func__, ioctl_data->opcode, (unsigned int)ioctl_data->idn);
  4381          err = -EINVAL;
  4382  out_release_mem:
  4383          kfree(ioctl_data);
  4384          kfree(desc);
  4385  out:
  4386          return err;
  4387  }

regards,
dan carpenter

             reply	other threads:[~2015-04-09 20:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-09 20:17 Dan Carpenter [this message]
2015-04-13 10:55 ` ufs: add ioctl interface for query request Gilad Broner
2015-04-13 11:01   ` Dan Carpenter

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=20150409201755.GA9651@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=draviv@codeaurora.org \
    --cc=linux-scsi@vger.kernel.org \
    /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.