kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: 'Greg KH' <greg@kroah.com>
To: rdq@metamail.co
Cc: kernelnewbies@kernelnewbies.org
Subject: Re: debugfs question ...
Date: Wed, 1 May 2019 16:35:38 +0200	[thread overview]
Message-ID: <20190501143538.GA30079@kroah.com> (raw)
In-Reply-To: <014a01d50028$ce098580$6a1c9080$@metamail.co>

On Wed, May 01, 2019 at 03:18:51PM +0100, rdq@metamail.co wrote:
> Hello and thanks,
> 
> > >
> > > The pattern for the implementation is (AFAICT) right out of  the book
> > >
> > 
> > You are returning a "short" read, and then disallowing ppos to be set to a
> > non-zero value?  That's a recipie for disaster :(
> > 
> > Also, you allow userspace to allocate as much memory as it asks for?
> > Not good :(
> 
> Well I started here:
> https://github.com/torvalds/linux/blob/v4.14/sound/soc/soc-core.c#L251.
> 
> > Why not just use the simple_read_from_buffer() call?  That handles all of
> the
> > "housekeeping" for you, and your function can be _much_ simpler.
> 
> Possibly but it does not explain why returning 0 means cat displays nothing

You returned nothing, so what is cat supposed to do?  It worked
properly.

> and returning the string length causes it to loop indefinitely. 

You have to give userspace what it asks for, don't ignore the ppos
variable.

Have you tried writing a userspace program that does a correct read()
call on a file or device node like this?  You have to keep reading until
you get all of the data you asked for, or the kernel returns
end-of-file, or an error happens.  You don't just do a single read and
hope for the best :)

> My example now reduced to:
> 
> 	int ret;
> 	char buf[64];
> 	int n = snprintf(buf,64,"Hello World\n");
> 	if (n < 0)
> 		return -EINVAL;
> 	// zero on success
> 	ret = copy_to_user(user_buf,buf,n);
> 	*ppos += n;	
> 	return ret;
> 
> Is this the canonical form?

Not at all, that's a recipe for disaster.

> Using a 4.14 kernel, cat displays nothing. Or has something changed?
> If so mighty odd as there are ancient example as well as the new:
> 
> https://sites.google.com/site/linuxkernel88/sample-code/writing-a-character-driver in char_device_read()

A char device is MUCH different from a file interface that you are using
here.  It kind of looks the same, but it has some subtle ways in which
things can go sideways very easily.

Also, that code is really really wrong, if that char_device_read()
function in the above link was ever submitted to me, I would have a
field day with it.  Nothing like bad random internet examples to steer
people in the wrong direction :(

> https://github.com/torvalds/linux/blob/master/drivers/base/regmap/regmap-debugfs.c#L254

That one is not "quite" like your example here either.

You have the option to handle all of the fun corner cases and error
values by writing it yourself, or just use the helper function I pointed
you at which does all of what you need to do, correctly.  To do
something in the middle is to guarantee that something will not work
properly with some sort of userspace program, or be totally insecure.

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

  parent reply	other threads:[~2019-05-01 14:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 11:45 debugfs question rdq
2019-04-30 12:07 ` Greg KH
     [not found]   ` <014a01d50028$ce098580$6a1c9080$@metamail.co>
2019-05-01 14:35     ` 'Greg KH' [this message]
2019-04-30 12:11 ` Martin Christian
2019-05-01 12:06   ` rdq

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=20190501143538.GA30079@kroah.com \
    --to=greg@kroah.com \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=rdq@metamail.co \
    /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 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).