[uml-devel] Patch for buffer overrun in serial/console device logic

[uml-devel] Patch for buffer overrun in serial/console device logic

[Doug Dumitru]

I have been having trouble with buffer overruns when talking to pty 
devices.  My boot config directs serial device with syntax like:

   ... ssl0=tty:/dev/pty/pty1009 ...

This sets up a serial line to talk with a pty device.  We don't use the 
ptmx interface so that we can definitavely know which device to talk to 
from the outside.

The problem we are seeing is that outbound data gets overrun.  If you 
send 100K out the stream, you tend to see the first 28K or so followed 
by a big gap followed by the last 3K or so of the stream.  We have 
tested this with the ptmx syntax/logic and the problem still seems to be 
there.  Also, if you test this with a very fast consumer (such as a 
local cat from the device), the problem does not manifest itself.  UML 
must be able to produce data faster than the outside world can consume it.

After a lot of looking, it appears that the line_write routine in line.c 
does not check to see if the buffer has enough room.  The 
buffer_data(...) routine does not appear to check for available space at 
all and just overwrites old data.

Here is the patch that appears to correct the issue:




--- ../linux-2.4.22/arch/um/drivers/line.c      Tue Oct  7 10:25:20 2003
+++ arch/um/drivers/line.c      Tue Oct  7 10:27:06 2003
@@ -97,6 +97,7 @@
         char *new;
         unsigned long flags;
         int n, err, i;
+       int room;

         if(tty->stopped) return 0;

@@ -115,6 +116,10 @@
         i = minor(tty->device) - tty->driver.minor_start;
         line = &lines[i];

+       room = line_write_room(tty);
+       if ( !room ) return -EAGAIN;
+       if ( room < len ) len = room;
+
         down(&line->sem);
         if(line->head != line->tail){
                 local_irq_save(flags);




We have this running in our 2.4.22 build, but would like to see it (or 
something equivelent) in the release so that we can take patches cleanly.

We also have a much messier, in-house patch that we apply to hostfs.c so 
that root mounts will correctly honor ownerships and rights.  Is anyone 
else working on this.  The 2.4.20-5um release appears to have a lot of 
missing stuff in this area, but if someone else is re-working this, I 
will put my patch down for a while and wait.  Otherwise, I will clean it 
up and submit it.

--------------------------------------------------------------------
Doug Dumitru     800-470-2756     (610-237-2000)
EasyCo LLC       doug@easyco.com  http://easyco.com
--------------------------------------------------------------------
D3, U2, jBase Virtual Servers.    Off-site backup over the internet.
Develop/test/deploy from $20/mo.  Fast, secure, cheaper than tape.
http://mirroredservers.com        http://mirroredbackup.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Re: Patch for buffer overrun in serial/console device logic

[Jeff Dike]

doug@easyco.com said:
> We also have a much messier, in-house patch that we apply to hostfs.c
> so  that root mounts will correctly honor ownerships and rights.  

Does this involve some sort of list on the side which keeps track of ownership
and permissions of the host files?  If so, hostfs has needed something like
this for a long time.

> Is anyone  else working on this.

Not that I know of.  It comes up every once in a while, but no one has actually
written any code.

				Jeff



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Re: Patch for buffer overrun in serial/console device logic

[Doug Dumitru]

Jeff Dike wrote:

> doug@easyco.com said:
> 
>>We also have a much messier, in-house patch that we apply to hostfs.c
>>so  that root mounts will correctly honor ownerships and rights.  
> 
> 
> Does this involve some sort of list on the side which keeps track of ownership
> and permissions of the host files?  If so, hostfs has needed something like
> this for a long time.
> 
> 
>>Is anyone  else working on this.
> 
> 
> Not that I know of.  It comes up every once in a while, but no one has actually
> written any code.
> 
> 				Jeff

The in-house patch that we are running assumes that you boot UML as 
root, although I have considered a SUID root variant.

It looks at numeric UID/GIDs and maintains all of these all the way down 
to the user IO calls.  Thus the files created in the hostfs have 
parellel UID/GID values and priviledges.

The code itself involves a lot of extra parameters from kernel to user 
space as things like the current user aren't propogated down.  I 
personally think that our current patch set is "100% ugly" and would not 
consider posting it as-is.  If people are interested in transparent 
numeric UID/GID to hostfs, then I would be happy to clean up what we 
have and submit it.

Our reason for doing the patch in-house is that we have added UML kernel 
code that supports an in-house application that causes hostfs to keep a 
"journel" of file IO transactions and posts this journel to a host pipe. 
  We then have host daemons (outside of the virtual) that move this 
journel to a distant machine achieving remote filesystem replication. 
We use this for high-reliability mail and web services.

--------------------------------------------------------------------
Doug Dumitru     800-470-2756     (610-237-2000)
EasyCo LLC       doug@easyco.com  http://easyco.com
--------------------------------------------------------------------
D3, U2, jBase Virtual Servers.    Off-site backup over the internet.
Develop/test/deploy from $20/mo.  Fast, secure, cheaper than tape.
http://mirroredservers.com        http://mirroredbackup.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Patch for buffer overrun in serial/console device logic

[Doug Dumitru]

Hannes Schulz wrote:

> [...]
> 
>> We also have a much messier, in-house patch that we apply to hostfs.c 
>> so that root mounts will correctly honor ownerships and rights.  Is 
>> anyone else working on this.  The 2.4.20-5um release appears to have a 
>> lot of missing stuff in this area, but if someone else is re-working 
>> this, I will put my patch down for a while and wait.  Otherwise, I 
>> will clean it up and submit it.
>>
> 
> I did such a patch (see below). How does yours fix the problem ?
> Unfortunately I have another problem with hostfs: munmap does not write 
> out the changes to the file. I looked at the code but I am afraid this 
> is beyound me.

[ ... snipped ... ]

My patch was quite a bit more involved.

The problem that I saw was that the actual IO operations in 
hostfs_user.c were always being executed as the user that start UML.  In 
order to get all of the UID/GIDs to map transparently, you have to do 
setegid(...) and seteuid(...) operations around all of the IO operations 
that have the potential to create a file or directory.  This includes 
open and a bunch of others.  Unfurtunately, the calling UID/GID is never 
visible to the functons in hostfs_user.c, so hostfs_kern.c needs a new 
call (or new parameters) to pass this down.  Umask also needs to travel 
down to hostfs_user.c.

My patch is very tangled in with my "transaction journel" that we use 
for replication.  The replication stuff is very "non trivial" to 
implement from a setup point of view, so I don't want to give that to 
anyone at this point.  Plus, the patch as stands has lots of debugging 
lines commented out and is just plain ugly.  My reputation would not be 
served well to show it to anyone at this point.  My real question was 
whether there is a need to more transparent UID/GID/UMASK processing to 
hostfs (with the caveat that UML must boot root or at least SUID root).


--------------------------------------------------------------------
Doug Dumitru     800-470-2756     (610-237-2000)
EasyCo LLC       doug@easyco.com  http://easyco.com
--------------------------------------------------------------------
D3, U2, jBase Virtual Servers.    Off-site backup over the internet.
Develop/test/deploy from $20/mo.  Fast, secure, cheaper than tape.
http://mirroredservers.com        http://mirroredbackup.com



-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Patch for buffer overrun in serial/console device logic

[Jeff Dike]

doug@easyco.com said:
> The code itself involves a lot of extra parameters from kernel to user
>  space as things like the current user aren't propogated down.  I
> personally think that our current patch set is "100% ugly" and would
> not  consider posting it as-is.  If people are interested in
> transparent  numeric UID/GID to hostfs, then I would be happy to clean
> up what we  have and submit it. 

OK, that ain't the way to do it.  Anything that involves passing a parallel
set of creds through VFS will cause Al Viro to lop my head off.  Since I'm
somewhat attached to it, I will not propose such a thing, no matter how cleaned
up it is.

What would work is to store the creds in a separate container of some sort
on the host, and reference that inside hostfs when doing permission checks.

This is more or less what UMSDOS does, from what I understand, and it keeps
the nastiness contained within hostfs.

That journalling is a neat idea, BTW.

				Jeff



-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Patch for buffer overrun in serial/console device logic

[Doug Dumitru]

Jeff Dike wrote:
> doug@easyco.com said:
> 
>>The code itself involves a lot of extra parameters from kernel to user
>> space as things like the current user aren't propogated down.  I
>>personally think that our current patch set is "100% ugly" and would
>>not  consider posting it as-is.  If people are interested in
>>transparent  numeric UID/GID to hostfs, then I would be happy to clean
>>up what we  have and submit it. 
> 
> 
> OK, that ain't the way to do it.  Anything that involves passing a parallel
> set of creds through VFS will cause Al Viro to lop my head off.  Since I'm
> somewhat attached to it, I will not propose such a thing, no matter how cleaned
> up it is.

There are actually a couple of sets of issues here.

1.  The "_user" stuff needs to get to UID, GID, and UMASK.

2.  When the "_user" stuff does IO, it should "become" the users in 
UID/GID.  This way permissions "just work" and you don't have to clean 
anything up after the fact.

Addressing (1), I could always just transition a pointer to "current" 
and really open up a can of worms.

This breaks just about every rule of "object orientation", but on the 
other hand, file systems usually do have access to the process state, so 
maybe this is the "correct" approach.

Doing a seteuid(...) ... around the IO calls is actually pretty clean. 
I implemented it with a "change" / "unchange" set of helper routines and 
it pretty much worked first shot.

> What would work is to store the creds in a separate container of some sort
> on the host, and reference that inside hostfs when doing permission checks.
> 
> This is more or less what UMSDOS does, from what I understand, and it keeps
> the nastiness contained within hostfs.
> 
> That journalling is a neat idea, BTW.
> 
> 				Jeff

--------------------------------------------------------------------
Doug Dumitru     800-470-2756     (610-237-2000)
EasyCo LLC       doug@easyco.com  http://easyco.com
--------------------------------------------------------------------
D3, U2, jBase Virtual Servers.    Off-site backup over the internet.
Develop/test/deploy from $20/mo.  Fast, secure, cheaper than tape.
http://mirroredservers.com        http://mirroredbackup.com



-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Re: Patch for buffer overrun in serial/console device logic

[BlaisorBlade]

Alle 03:49, sabato 11 ottobre 2003, Jeff Dike ha scritto:
> doug@easyco.com said:
> > The code itself involves a lot of extra parameters from kernel to user
> >  space as things like the current user aren't propogated down.  I
> > personally think that our current patch set is "100% ugly" and would
> > not  consider posting it as-is.  If people are interested in
> > transparent  numeric UID/GID to hostfs, then I would be happy to clean
> > up what we  have and submit it.
>
> OK, that ain't the way to do it.  Anything that involves passing a parallel
> set of creds through VFS will cause Al Viro to lop my head off.  Since I'm
> somewhat attached to it, I will not propose such a thing, no matter how
> cleaned up it is.
>
> What would work is to store the creds in a separate container of some sort
> on the host, and reference that inside hostfs when doing permission checks.
>
> This is more or less what UMSDOS does, from what I understand, and it keeps
> the nastiness contained within hostfs.
>
> That journalling is a neat idea, BTW.
This are my very humble opinions, just my 2 cents, however let's go.
1)a solution involving a setuid UML is just wrong. Something lighter is 
obviously welcome.
2) so, on the host, all files must have the same permissions, and the same 
owner, when you use hostfs as rootfs. I.e., the uid and the gid will have to 
be the same as the ones UML runs with. And the permission will have to be 
fixed ones. For instance, for files we have fumask=644 by default, and it can 
be set to anything not stricter than 600; for dirs, dumask=755 by default, 
and it mustn't be stricter than 700. If and only if you want to have fun, you 
can check if a file has u+x permission and give u+x(or a+x, it's matter of 
taste/security) on the host. These params can be set on the linux command 
line; a mount option can exist, but it must be "bounded" by the boot 
option(the same way you can specify the rootfs option and then restrict even 
more the mount).
Then, there can be UMSDOS-like stuff.
This is the only way to make possible things such as a file that inside UML 
has perms=000 and is readable by root. In this case, hostfs is a true 
filesystem.
3)But, there is a separate issue: accessing the host files, i.e. when you want 
to watch on files existing on the host(so you can't change freely their 
permission). This is a completely different case! Here you can't even suppose 
you can write the UMSDOS like permission-holding file.
So here root has limited permission. My idea here is to make things follow 
some extended semantic, like the 2.6 security models: neither there root has 
all powers, or seeming a NFS mount(neither there root has full permissions). 
It's just a start of idea, but *could* be interesting.
-- 
cat <<EOSIGN
Paolo Giarrusso, aka Blaisorblade
Linux Kernel 2.4.21/2.6.0-test on an i686; Linux registered user n. 292729
EOSIGN



-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Re: Patch for buffer overrun in serial/console device logic

[Jeff Dike]

doug@easyco.com said:
> After a lot of looking, it appears that the line_write routine in
> line.c  does not check to see if the buffer has enough room.  The
> buffer_data(...) routine does not appear to check for available space
> at  all and just overwrites old data. 

Indeed.  Thanks for chasing this down.  I fixed it, but in a different way.

				Jeff



-------------------------------------------------------
This SF.Net email sponsored by: ApacheCon 2003,
16-19 November in Las Vegas. Learn firsthand the latest
developments in Apache, PHP, Perl, XML, Java, MySQL,
WebDAV, and more! http://www.apachecon.com/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel