Date: Fri, 24 Jul 1998 22:38:42 +1000 From: Bruce Evans <bde@zeta.org.au> To: green@zone.baldcom.net, jkh@time.cdrom.com, luoqi@watermarkgroup.com Cc: bright@hotjobs.com, freebsd-current@FreeBSD.ORG, joelh@gnu.org Subject: Re: vn subsystem Message-ID: <199807241238.WAA31742@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>> > mount /dev/vn0 /msdos (oops, should this matter anyway?)
>> > mount_msdos /dev/vn0 /msdos
>>
>> Try it without the bad mount first. This sounds an awful lot like
>> another PR which has nothing to do with vn (e.g. it would do the *same*
>> thing if you tried to do this with a non-vn device).
>>
>> - Jordan
>>
>I took a look at this problem, I found there're some bugs in VMIO code
>when dealing with buf at a non-page-aligned blkno, e.g. reading one page
>size of data at block 1 from a block device, as Brian Feldman's core dump
>shows, since the buf does not start at a page bounary, it should span
>two pages, yet only one page is allocated in the current code, and
>subsequent write to the 2nd page would result in a fault. I took a shot
>at fixing this problem, resulted in the patch below. Would any knowledgeable
>person please take a look at the patch? I've found no ill effect so far
I don't think the bug can be fixed at this level. The size of a B_VMIO
buffer is supposed to be a multiple of PAGE_SIZE. Smaller buffers are
supposed to be malloced. msdosfs_mount() only gets as far as having
misaligned blkno's because of incomplete cleanup from a previous (usually
failed) mount. (IIRC, vp->v_object (where vp is the vnode for the block
device) is not cleared even when all references to vp go away, and this
somehow causes use of a stale block size.)
I think the correct fix is to get rid of the stale v_object and improve
the block size guessing (don't guess).
I'm not sure what the deblocking stuff in allocbuf() is for. Is it only
for NFS? FFS with its >= 4K block size never goes near any of the
complications there.
>Index: vfs_bio.c
>===================================================================
>RCS file: /fun/cvs/src/sys/kern/vfs_bio.c,v
>retrieving revision 1.167
>diff -u -r1.167 vfs_bio.c
>--- vfs_bio.c 1998/07/13 07:05:55 1.167
>+++ vfs_bio.c 1998/07/24 08:14:34
>@@ -2192,15 +2192,14 @@
> vm_ooffset_t soff, eoff;
>
> soff = off;
>- eoff = off + min(PAGE_SIZE, bp->b_bufsize);
>+ eoff = min(off + PAGE_SIZE, bp->b_offset + bp->b_bufsize);
This may overflow. off and bp->b_offset have type off_t, while min()
truncates to u_int. Many other new and old uses of [il]min() and
[il]max() may overflow for possible but not so likely combinations of
type sizes.
>@@ -2285,18 +2284,21 @@
> panic("vfs_clean_pages: no buffer offset");
> #endif
>
>- for (i = 0; i < bp->b_npages; i++, foff += PAGE_SIZE) {
>+ for (i = 0; i < bp->b_npages; i++) {
> vm_page_t m = bp->b_pages[i];
> vfs_page_set_valid(bp, foff, i, m);
>+ foff = trunc_page(foff + PAGE_SIZE);
This may overflow. foff has type vm_ooffset_t, while trunc_page() is only
designed to work on core addresses, and in fact truncates to u_int.
> }
> }
> }
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199807241238.WAA31742>