Patch #1754 has been updated. 

Project: 
Category: libstore
Status: Closed
Summary: store_{read,write} abort program when LEN is not aligned

Follow-Ups:

Date: Wed 07/23/2003 at 13:35
By: ogi

Comment:
* Copyright years are updated.
-------------------------------------------------------

Date: Thu 07/31/2003 at 22:46
By: marcus

Comment:
The assert is there to catch bugs in libstore using programs, which usually only 
access store blocks with aligned boundaries.

Do you have an application which doesn't do this, or did you see a Hurd program using 
libstore in a way that triggers the assertion?

-------------------------------------------------------

Date: Fri 08/01/2003 at 09:15
By: ogi

Comment:
I hit this problem while patching e2fsprogs to work with libstore.  e2fsprogs uses 
some unaligned reads from block devices.

If libstore was internal library in the Hurd, I would agree with you.  But native Hurd 
programs (not in the Hurd distribution) can legally want to use the library, so 
libstore must behave correctly on invalid arguments.  For example, munmap returns 
EINVAL if address is not aligned, it doesn't throw assertion failure.  What is the 
difference between munmap and store_read so that store_read have to not return EINVAL 
on invalid arguments?

Moreover, every program that uses store_read must check if I/O errors occured.  So the 
EINVAL will always be caught ;-)
-------------------------------------------------------

Date: Fri 08/01/2003 at 12:41
By: marcus

Comment:
Why does e2fsprogs unaligned reads?  I would like to discuss what it does, 
independently of resolving this patch.

I agree that eventually we want to make libstore more robust  But before we can do 
that, we must check that all existing callers, at least those in the Hurd code, but 
preferably also parted etc, do check the return value properly.  Can you please do 
that?

-------------------------------------------------------

Date: Sat 08/02/2003 at 13:46
By: ogi

Comment:
In e2fsprogs, the I/O function that reads data is io_channel_read_blk. It accepts 
_number of blocks_.  But if this count is negative, then its negated value is accepted 
as _number of bytes_.   For example, this is used in 
e2fsprogs/lib/ext2fs/rw_bitmaps:read_bitmaps.  Of course, this is not a problem when 
you use Unix API.

I'm willing to check all the Hurd for calls to store_read and store_write, but I don't 
know any other program, except parted, that uses libstore.  If there is, please tell 
me.
-------------------------------------------------------

Date: Sat 08/02/2003 at 23:50
By: marcus

Comment:
I am confused.  Are you saying that e2fsprogs does read bytes rather than blocks from 
the store?  In that case an EINVAL would not help it, except make it fail properly 
instead aborting it.  Or is that only how you found out about this problem?

I don't think there is any other program than parted.  I am more worried about the 
Hurd itself.  If you can check its callers, then I am willing to put the change in, 
independently of what e2fsprogs does, for the reasons you gave.


-------------------------------------------------------

Date: Sun 08/03/2003 at 15:58
By: ogi

Comment:
This is the way how I found the problem.

When caller of io_channel_read_blk wants to read bytes (not blocks), the number of 
bytes is rounded to store->block_size, and after store_read, only the desired bytes 
are copied to the caller-supplied buffer.  Fortunately, the starting offset is always 
measured in blocks, and never in bytes.
-------------------------------------------------------

Date: Sun 08/03/2003 at 16:28
By: marcus

Comment:
I am still confused.  If e2fsprogs does the right thing wrt reading only blocks, how 
did you find out about the assertion?  Did you actually see the assertion being 
triggered, or was the patch the result of source studying?

-------------------------------------------------------

Date: Mon 08/04/2003 at 09:40
By: ogi

Comment:
The original e2fsprogs uses Unix API, so there is no problem with it.

The patched e2fsprogs (with my patch) passes valid arguments when either reading 
blocks or reading bytes (as I told earlier).

When I was making the patch for e2fsprogs, e2fsck hangs or terminates or something 
like that, I can't recall exactly.  So I started debugging the code, and the assertion 
was the place where the things messed up.  Yes, the assertion failure notice didn't 
show up, but definately this was the place where the process hangs/terminates or 
whatever it was.

I will check why the text of the assertion wasn't printed, as this can reveal another 
bug. 
-------------------------------------------------------

Date: Tue 08/05/2003 at 00:01
By: marcus

Comment:
Ok.  I applied the patch.  Thanks!

-------------------------------------------------------

-------------------------------------------------------
For more info, visit:

http://savannah.gnu.org/patch/?func=detailpatch&patch_id=1754&group_id=30

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd

Reply via email to