lkml.org 
[lkml]   [2019]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V3 04/12] misc: xilinx_sdfec: Add open, close and ioctl
    On Sat, Apr 27, 2019 at 11:04:58PM +0100, Dragan Cvetic wrote:
    > +static int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
    > +{
    > + struct xsdfec_dev *xsdfec;
    > +
    > + xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
    > +
    > + if (!atomic_dec_and_test(&xsdfec->open_count)) {

    Why do you care about this?

    And do you really think it matters? What are you trying to protect from
    here?

    > + atomic_inc(&xsdfec->open_count);
    > + return -EBUSY;
    > + }
    > +
    > + fptr->private_data = xsdfec;
    > + return 0;
    > +}
    > +
    > +static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
    > +{
    > + struct xsdfec_dev *xsdfec;
    > +
    > + xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
    > +
    > + atomic_inc(&xsdfec->open_count);

    You increment a number when the device is closed?

    You are trying to make it hard to maintain this code over time :)


    > + return 0;
    > +}
    > +
    > +static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
    > + unsigned long data)
    > +{
    > + struct xsdfec_dev *xsdfec = fptr->private_data;
    > + void __user *arg = NULL;
    > + int rval = -EINVAL;
    > + int err = 0;
    > +
    > + if (!xsdfec)
    > + return rval;
    > +
    > + if (_IOC_TYPE(cmd) != XSDFEC_MAGIC)
    > + return -ENOTTY;
    > +
    > + /* check if ioctl argument is present and valid */
    > + if (_IOC_DIR(cmd) != _IOC_NONE) {
    > + arg = (void __user *)data;
    > + if (!arg) {
    > + dev_err(xsdfec->dev,
    > + "xilinx sdfec ioctl argument is NULL Pointer");

    You just created a way for userspace to spam the kernel log, please do
    not do that :(



    > + return rval;
    > + }
    > + }
    > +
    > + if (err) {
    > + dev_err(xsdfec->dev, "Invalid xilinx sdfec ioctl argument");
    > + return -EFAULT;

    Wrong error, you did not have a memory fault.

    Again, you just created a way to spam the kernel log by a user :(

    thanks,

    greg k-h

    \
     
     \ /
      Last update: 2019-05-02 19:23    [W:4.315 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site