Re: [patch] mg: Prevent out-of-bounds read when PATH="/:..."
Thank you for the diff.
> I looked for more instances of the pattern that lead to reading one byte
> before an allocated buffer in which(1) when PATH begins with "/:". I
> found only one, in the function csexists() in usr.bin/mg/cscope.c.
> + while ((dir = strsep(&path, ":")) != NULL) {
> + if (*dir== '\0')
> + *dir = '.';
>
> - dlen = strlen(dir);
> - while (dir[dlen-1] == '/')
> - dir[--dlen] = '\0'; /* strip trailing '/' */
> + dlen = strlen(dir);
> + while (dlen > 0 && dir[dlen-1] == '/')
> + dir[--dlen] = '\0'; /* strip trailing '/' */
dlen could never be zero as we are replacing dir[0] with '.' if
it's an empty field but that has another problem of wrong strlen(3)
values due to improper NUL termination. The simple fix is to skip
empty fields in PATH which I committed.
>
> While at it, I replaced the manual length check before snprintf() with a
> check of the return value, and I replaced the space indents in the
> csexists() function with tabs. (The rest of the file uses tabs.)
> If that makes the diff inconvenient to check, I'll resubmit it without
> the indent change.
These changes are committed. It's a good idea to split diffs where
possible for ease of readability and review.
Re: [patch] mg: Prevent out-of-bounds read when PATH="/:..."
On Tue, Jan 19, 2016 at 12:35:27PM +0100, Sunil Nimmagadda wrote: > > - dlen = strlen(dir); > > - while (dir[dlen-1] == '/') > > - dir[--dlen] = '\0'; /* strip trailing '/' */ > dlen could never be zero as we are replacing dir[0] with '.' if > it's an empty field but that has another problem of wrong strlen(3) > values due to improper NUL termination. The simple fix is to skip > empty fields in PATH which I committed. Actually, the problem my diff was supposed to address is not empty fields, but fields containing a slash and nothing else. Then, dir[0] == '/' and dir[1] == '\0', so dlen == 1, and the while-loop quoted above runs, and *decrements dlen* to 0. Then, to check if the loop condition is still true, the program reads dir[-1]. If the slash-only field is at the beginning of the path (e.g., PATH = "/:/bin:...") this means reading path[-1]. > These changes are committed. It's a good idea to split diffs where > possible for ease of readability and review. I'll do so in the future. Thanks for taking the time to check it.
Re: [patch] mg: Prevent out-of-bounds read when PATH="/:..."
> On Tue, Jan 19, 2016 at 12:35:27PM +0100, Sunil Nimmagadda wrote: > > > > - dlen = strlen(dir); > > > - while (dir[dlen-1] == '/') > > > - dir[--dlen] = '\0'; /* strip trailing '/' */ > > > dlen could never be zero as we are replacing dir[0] with '.' if > > it's an empty field but that has another problem of wrong strlen(3) > > values due to improper NUL termination. The simple fix is to skip > > empty fields in PATH which I committed. > > Actually, the problem my diff was supposed to address is not empty > fields, but fields containing a slash and nothing else. Then, > dir[0] == '/' and dir[1] == '\0', so dlen == 1, and the while-loop > quoted above runs, and *decrements dlen* to 0. Then, to check if the > loop condition is still true, the program reads dir[-1]. If the > slash-only field is at the beginning of the path (e.g., > PATH = "/:/bin:...") this means reading path[-1]. ah right, sorry, I misread your diff. Ok to commit? Index: cscope.c === RCS file: /cvs/src/usr.bin/mg/cscope.c,v retrieving revision 1.15 diff -u -p -r1.15 cscope.c --- cscope.c19 Jan 2016 11:39:06 - 1.15 +++ cscope.c19 Jan 2016 13:38:06 - @@ -614,7 +614,7 @@ csexists(const char *cmd) continue; dlen = strlen(dir); - while (dir[dlen-1] == '/') + while (dlen > 0 && dir[dlen-1] == '/') dir[--dlen] = '\0'; /* strip trailing '/' */ len = snprintf(fname, sizeof(fname), "%s/%s", dir, cmd);
vmctl create nit-picks
Hi,
below some comments regarding the vmctl disk image creation code.
Index: main.c
===
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 main.c
--- main.c 5 Jan 2016 16:25:34 - 1.13
+++ main.c 10 Jan 2016 15:30:55 -
@@ -421,9 +421,6 @@ ctl_create(struct parse_result *res, int
fprintf(stderr, "missing size argument\n");
ctl_usage(res->ctl);
}
- res->size /= 1024 / 1024;
- if (res->size < 1)
- errx(1, "specified image size too small");
ret = create_imagefile(paths[0], res->size);
if (ret != 0) {
errno = ret;
The division is a no-op: 1024 / 1024 is evaluated first, resulting in
res->size /= 1. At that point res->size has already been converted to
megabytes by parse_size().
The condition (res->size < 1) can never be true, because in that case
parse_size() would have returned non-zero, triggering the
errx(1, "invalid size: %s", optarg) in ctl_create().
Index: vmctl.c
===
RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 vmctl.c
--- vmctl.c 14 Dec 2015 06:59:07 - 1.10
+++ vmctl.c 10 Jan 2016 15:30:55 -
@@ -376,7 +376,7 @@ vm_console(struct vmop_info_result *list
*
* Parameters:
* imgfile_path: path to the image file to create
- * imgsize : size of the image file to create
+ * imgsize : size of the image file to create (in MB)
*
* Return:
* EEXIST: The requested image file already exists
@@ -387,8 +387,6 @@ int
create_imagefile(const char *imgfile_path, long imgsize)
{
int fd, ret;
- off_t ofs;
- char ch = '\0';
/* Refuse to overwrite an existing image */
fd = open(imgfile_path, O_RDWR | O_CREAT | O_TRUNC | O_EXCL,
@@ -396,19 +394,11 @@ create_imagefile(const char *imgfile_pat
if (fd == -1)
return (errno);
- ofs = (imgsize * 1024 * 1024) - 1;
-
- /* Set fd pos at desired size */
- if (lseek(fd, ofs, SEEK_SET) == -1) {
- ret = errno;
- close(fd);
- return (ret);
- }
-
- /* Write one byte to fill out the extent */
- if (write(fd, &ch, 1) == -1) {
+ /* Extend to desired size */
+ if (ftruncate(fd, (off_t)imgsize * 1024 * 1024) == -1) {
ret = errno;
close(fd);
+ unlink(imgfile_path);
return (ret);
}
lseek() + write() can be replaced by a slightly shorter ftruncate()
call. Note that using ftruncate() to extend a file is not portable
(Posix allows either zero-filling until the given size is reached, or
alternatively erroring out.), but that shouldn't be a proble as vmm
isn't cross-platform either.
I think it would be nice to unlink() the image file when extending it
fails for consistency with the other error case (the file can't be
created).
cheers,
natano
vmx: vmxnet3_load_mbuf will still do the wrong thing
Hi, We've just run into a vmx panic and code inspection revealed that my previous diff contained a mistake, the pullup operation is called on a wrong mbuf chain. I apologize for overlooking this issue. We're not 100% certain that this fixes our exact problem yet since we can't reproduce it at will, but the diff appears correct to us. Please test and report any problems. Thanks! diff --git sys/dev/pci/if_vmx.c sys/dev/pci/if_vmx.c index 2a3367c..7710987 100644 --- sys/dev/pci/if_vmx.c +++ sys/dev/pci/if_vmx.c @@ -1121,11 +1121,11 @@ vmxnet3_load_mbuf(struct vmxnet3_softc *sc, struct vmxnet3_txring *ring, return (-1); ip = (struct ip *)(n->m_data + offp); hlen += ip->ip_hl << 2; - *mp = m_pullup(m, hlen + csum_off + 2); + *mp = m_pullup(n, hlen + csum_off + 2); if (*mp == NULL) return (-1); m = *mp; }
Re: cd9660 uiomove() conversion
>
> Diff reads good. ok?
ok kettenis@
> Some thoughts (mostly for myself) inline.
>
> Martin Natano wrote:
> > Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c.
> >
> > cheers,
> > natano
> >
> > Index: isofs/cd9660/cd9660_vnops.c
> > ===
> > RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v
> > retrieving revision 1.73
> > diff -u -p -r1.73 cd9660_vnops.c
> > --- isofs/cd9660/cd9660_vnops.c 11 Dec 2015 11:25:55 - 1.73
> > +++ isofs/cd9660/cd9660_vnops.c 1 Jan 2016 17:45:50 -
> > @@ -227,7 +227,8 @@ cd9660_read(void *v)
> > daddr_t lbn, rablock;
> > off_t diff;
> > int error = 0;
> > - long size, n, on;
> > + long size, on;
> > + size_t n;
> >
> > if (uio->uio_resid == 0)
> > return (0);
> > @@ -240,8 +241,7 @@ cd9660_read(void *v)
> >
> > lbn = lblkno(imp, uio->uio_offset);
> > on = blkoff(imp, uio->uio_offset);
> > - n = min((u_int)(imp->logical_block_size - on),
> > - uio->uio_resid);
> > + n = ulmin(imp->logical_block_size - on, uio->uio_resid);
>
> The subtraction can't overflow, because blkoff is basically a
> uio->uio_offset % imp->logical_block_size. ulmin() protects against
> truncation. The result is always positive and making n size_t is
> straightforward.
>
> > diff = (off_t)ip->i_size - uio->uio_offset;
> > if (diff <= 0)
> > return (0);
> > @@ -270,13 +270,13 @@ cd9660_read(void *v)
> > } else
> > error = bread(vp, lbn, size, &bp);
> > ci->ci_lastr = lbn;
> > - n = min(n, size - bp->b_resid);
> > + n = ulmin(n, size - bp->b_resid);
>
> bp->b_resid is supposed to be <= size because it's the
> remaining I/O to get the whole buf filled with size bytes.
> So bp->b_resid should be initialized with size initially
> and then only decrease.
>
> > if (error) {
> > brelse(bp);
> > return (error);
> > }
> >
> > - error = uiomovei(bp->b_data + on, (int)n, uio);
> > + error = uiomove(bp->b_data + on, n, uio);
>
> Straightforward with n being a size_t.
>
> > brelse(bp);
> > } while (error == 0 && uio->uio_resid > 0 && n != 0);
> > @@ -344,7 +344,7 @@ iso_uiodir(idp,dp,off)
> > }
> >
> > dp->d_off = off;
> > - if ((error = uiomovei((caddr_t)dp, dp->d_reclen, idp->uio)) != 0)
> > + if ((error = uiomove(dp, dp->d_reclen, idp->uio)) != 0)
>
> Straightforward. dp->d_reclen is unsigned.
>
> > return (error);
> > idp->uio_off = off;
> > return (0);
> > @@ -657,7 +657,7 @@ cd9660_readlink(void *v)
> > */
> > if (uio->uio_segflg != UIO_SYSSPACE ||
> > uio->uio_iov->iov_len < MAXPATHLEN) {
> > - error = uiomovei(symname, symlen, uio);
> > + error = uiomove(symname, symlen, uio);
>
> Straightforward, symlen is unsigned. Also did some checking that computation
> of symlen does not wrap around also.
>
> > pool_put(&namei_pool, symname);
> > return (error);
> > }
> >
>
>
Re: spec_vnops.c uiomove() conversion
> Date: Mon, 18 Jan 2016 19:12:01 +0100
> From: Stefan Kempf
>
> Martin Natano wrote:
> > Below the conversion to uiomove() for kern/spec_vnops.c. This diff
> > prevents truncation of uio_resid when passed to min().
>
> Looks good. Basically the same reasoning as with the cd9660 diff.
>
> ok?
ok kettenis@
> > Index: kern/spec_vnops.c
> > ===
> > RCS file: /cvs/src/sys/kern/spec_vnops.c,v
> > retrieving revision 1.84
> > diff -u -p -u -r1.84 spec_vnops.c
> > --- kern/spec_vnops.c 5 Dec 2015 10:11:53 - 1.84
> > +++ kern/spec_vnops.c 13 Jan 2016 19:12:40 -
> > @@ -202,7 +202,8 @@ spec_read(void *v)
> > daddr_t bn, nextbn, bscale;
> > int bsize;
> > struct partinfo dpart;
> > - int n, on, majordev;
> > + size_t n;
> > + int on, majordev;
> > int (*ioctl)(dev_t, u_long, caddr_t, int, struct proc *);
> > int error = 0;
> >
> > @@ -243,7 +244,7 @@ spec_read(void *v)
> > do {
> > bn = btodb(uio->uio_offset) & ~(bscale - 1);
> > on = uio->uio_offset % bsize;
> > - n = min((bsize - on), uio->uio_resid);
> > + n = ulmin((bsize - on), uio->uio_resid);
> > if (vp->v_lastr + bscale == bn) {
> > nextbn = bn + bscale;
> > error = breadn(vp, bn, bsize, &nextbn, &bsize,
> > @@ -251,12 +252,12 @@ spec_read(void *v)
> > } else
> > error = bread(vp, bn, bsize, &bp);
> > vp->v_lastr = bn;
> > - n = min(n, bsize - bp->b_resid);
> > + n = ulmin(n, bsize - bp->b_resid);
> > if (error) {
> > brelse(bp);
> > return (error);
> > }
> > - error = uiomovei((char *)bp->b_data + on, n, uio);
> > + error = uiomove((char *)bp->b_data + on, n, uio);
> > brelse(bp);
> > } while (error == 0 && uio->uio_resid > 0 && n != 0);
> > return (error);
> > @@ -290,7 +291,8 @@ spec_write(void *v)
> > daddr_t bn, bscale;
> > int bsize;
> > struct partinfo dpart;
> > - int n, on, majordev;
> > + size_t n;
> > + int on, majordev;
> > int (*ioctl)(dev_t, u_long, caddr_t, int, struct proc *);
> > int error = 0;
> >
> > @@ -331,14 +333,14 @@ spec_write(void *v)
> > do {
> > bn = btodb(uio->uio_offset) & ~(bscale - 1);
> > on = uio->uio_offset % bsize;
> > - n = min((bsize - on), uio->uio_resid);
> > + n = ulmin((bsize - on), uio->uio_resid);
> > error = bread(vp, bn, bsize, &bp);
> > - n = min(n, bsize - bp->b_resid);
> > + n = ulmin(n, bsize - bp->b_resid);
> > if (error) {
> > brelse(bp);
> > return (error);
> > }
> > - error = uiomovei((char *)bp->b_data + on, n, uio);
> > + error = uiomove((char *)bp->b_data + on, n, uio);
> > if (n + on == bsize)
> > bawrite(bp);
> > else
> >
> > cheers,
> > natano
> >
>
>
Re: 0 is UIO_USERSPACE
Martin Natano wrote: > 0 is the same as UIO_USERSPACE, but more readable. No binary change. Makes sense. The entire tree uses the symbolic enum values. These are the only places that use 0. > Index: ./arch/octeon/dev/amdcf.c > === > RCS file: /cvs/src/sys/arch/octeon/dev/amdcf.c,v > retrieving revision 1.1 > diff -u -p -u -r1.1 amdcf.c > --- ./arch/octeon/dev/amdcf.c 20 Jul 2015 19:44:32 - 1.1 > +++ ./arch/octeon/dev/amdcf.c 9 Jan 2016 17:56:04 - > @@ -548,7 +548,7 @@ amdcfioctl(dev_t dev, u_long xfer, caddr > auio.uio_iov = &aiov; > auio.uio_iovcnt = 1; > auio.uio_resid = fop->df_count; > - auio.uio_segflg = 0; > + auio.uio_segflg = UIO_USERSPACE; > auio.uio_offset = > fop->df_startblk * sc->sc_dk.dk_label->d_secsize; > auio.uio_procp = p; > Index: ./arch/octeon/dev/octcf.c > === > RCS file: /cvs/src/sys/arch/octeon/dev/octcf.c,v > retrieving revision 1.27 > diff -u -p -u -r1.27 octcf.c > --- ./arch/octeon/dev/octcf.c 20 Jul 2015 01:38:31 - 1.27 > +++ ./arch/octeon/dev/octcf.c 9 Jan 2016 17:56:09 - > @@ -616,7 +616,7 @@ octcfioctl(dev_t dev, u_long xfer, caddr > auio.uio_iov = &aiov; > auio.uio_iovcnt = 1; > auio.uio_resid = fop->df_count; > - auio.uio_segflg = 0; > + auio.uio_segflg = UIO_USERSPACE; > auio.uio_offset = > fop->df_startblk * wd->sc_dk.dk_label->d_secsize; > auio.uio_procp = p; > Index: ./dev/ata/wd.c > === > RCS file: /cvs/src/sys/dev/ata/wd.c,v > retrieving revision 1.119 > diff -u -p -u -r1.119 wd.c > --- ./dev/ata/wd.c26 Aug 2015 22:29:39 - 1.119 > +++ ./dev/ata/wd.c9 Jan 2016 17:56:10 - > @@ -836,7 +836,7 @@ wdioctl(dev_t dev, u_long xfer, caddr_t > auio.uio_iov = &aiov; > auio.uio_iovcnt = 1; > auio.uio_resid = fop->df_count; > - auio.uio_segflg = 0; > + auio.uio_segflg = UIO_USERSPACE; > auio.uio_offset = > fop->df_startblk * wd->sc_dk.dk_label->d_secsize; > auio.uio_procp = p; > > cheers, > natano >
Re: sis(4) induced uvm faults on net4801
Hi,
I have the same problem with an re(4) adapter and CARP.
My
post is in the bugs mail list (UVM Fault with CARP on re(4), 2016-01-17
09:15)
Fabrice
Le 2016-01-07 12:36, Nathanael Rensen a écrit :
>
tl;dr: Zero-length packets from sis(4) on net4801 result in negative
>
length mbufs causing uvm faults.
>
> I have observed uvm faults shortly
after bringing up a sis(4) interface on
> a Soekris net4801:
>
>
uvm_fault(0xd3adbbf0, 0xd3ee5000, 0, 1) -> e
> kernel: page fault trap,
code=0
> Stopped at memcpy+0x13: repe movsl (%esi),%es:(%edi)
>
> ddb>
trace
> memcpy(d6025e00,1a280e,3b9aca00,2,1) at memcpy+0x13
>
m_copym2(d6025e00,e,3b9aca00,2) at m_copym2+0x19
>
bridge_m_dup(d6025e00,d3eed000,f13c1d78,d02a0d4b) at bridge_m_dup+0x2f
>
bridge_localbroadcast(d3eed000,d3ee0c00,f13c1dda,d6025e00) at
bridge_localbroad
> cast+0x47
>
bridge_broadcast(d3eed000,d3d3d834,f13c1dda,d6025e00,d3d3d834) at
bridge_broadc
> ast+0xaf
>
bridgeintr_frame(d3eed000,d3d3d834,d6025e00,d3ed9700) at
bridgeintr_frame+0x1ed
>
>
bridge_process(d3d3d834,d6025e00,f13c1e6c,f13c1e6c,d6025c00) at
bridge_process+
> 0x257
>
bridgeintr(d029312e,d3b22b20,d6025c00,f13c1ea8,d3d5c800) at
bridgeintr+0x54
> netintr(0,d020224c,0,f13c1efc) at netintr+0x47
>
softintr_dispatch(1) at softintr_dispatch+0x6c
> Xsoftnet() at
Xsoftnet+0x12
> --- interrupt ---
> 0:
>
> ddb> show mbuf 0xd6025e00
>
mbuf 0xd6025e00
> m_type: 1 m_flags: b
> m_next: 0x0 m_nextpkt: 0x0
>
m_data: 0xd3d42000 m_len: 4294967292
> m_dat: 0xd6025e14 m_pktdat:
0xd6025e44
> m_ptkhdr.ph_ifidx: 1 m_pkthdr.len: -4
> m_ptkhdr.ph_tags:
0x0 m_pkthdr.ph_tagsset: 0
> m_pkthdr.ph_flowid: 0
>
m_pkthdr.csum_flags: 0
> m_pkthdr.ether_vtag: 0 m_ptkhdr.ph_rtableid:
0
> m_pkthdr.pf.statekey: 0x0 m_pkthdr.pf.inp 0x0
> m_pkthdr.pf.qid: 0
m_pkthdr.pf.tag: 0
> m_pkthdr.pf.flags: 0
> m_pkthdr.pf.routed: 0
m_pkthdr.pf.prio: 3
> m_ext.ext_buf: 0xd3d42000 m_ext.ext_size: 2048
>
m_ext.ext_free: 0xd027a717 m_ext.ext_arg: 0xd3b0050c
>
m_ext.ext_nextref: 0xd6025e00 m_ext.ext_prevref: 0xd6025e00
>
> # dmesg
| grep sis0
> sis0 at pci0 dev 6 function 0 vendor 0x100b product 0x0020
rev 0x00, DP83816A: irq 10, address 00:00:24:c4:fe:3c
> nsphyter0 at
sis0 phy 0: DP83815 10/100 PHY, rev. 1
>
> I traced this to the
DP83816A sometimes producing a small number of zero-
> length packets
soon after initialisation. The sis(4) driver unconditionally
> subtracts
ETHER_CRC_LEN from the packet length. When the packet length is
> zero
this results in a negative length mbuf.
>
> #define SIS_RXBYTES(x)
>
((letoh32((x)->sis_ctl) & SIS_CMDSTS_BUFLEN) - ETHER_CRC_LEN)
>
>
total_len = SIS_RXBYTES(cur_rx);
>
> m->m_pkthdr.len = m->m_len =
total_len;
>
> I haven't been able to nail down the root cause of this
behaviour. It is
> not a recently introduced issue. For me this happens
commonly enough that
> it is readily reproduced, but the zero-length
packets do not occur on
> every boot and not every zero-length packet
results in a uvm fault.
>
> A web search didn't turn up any similar
reports so perhaps there is
> something unusual about my situation.
Maybe having sis0 on a bridge
> introduces a code path that increases
the likelihood of a uvm_fault in
> response to a negative length mbuf.
>
> The descriptor cmdsts for these zero-length packets is
consistently:
>
> 0xd000 = SIS_CMDSTS_OWN | SIS_CMDSTS_MORE |
SIS_CMDSTS_CRC
>
> I found the following comment in the DP83816A
datasheet:
>
> Care must be taken when setting DRTH to a value lower
than the number
> of bytes needed to determine if packet should be
accepted or rejected.
> In this case, the packet might be rejected after
the bus master
> operation to begin transferring the packet into memory
has begun. When
> this occurs, neither the OK bit or any error status
bit in the
> descriptor's cmdsts will be set.
>
> sis(4) sets DRTH to
64 bytes, which is sufficient for packet identification.
> In the end,
maybe this is just buggy DP83816A behaviour.
>
> In any case the
following diff works around the problem for me.
>
> Nathanael
>
>
Index: if_sisreg.h
>
===
>
RCS file: /cvs/src/sys/dev/pci/if_sisreg.h,v
> retrieving revision
1.34
> diff -u -p -r1.34 if_sisreg.h
> --- if_sisreg.h 11 Feb 2015
21:36:02 - 1.34
> +++ if_sisreg.h 6 Jan 2016 14:33:11 -
> @@
-343,8 +343,7 @@ struct sis_desc {
> #define SIS_LASTDESC(x)
(!(letoh32((x)->sis_ctl) & SIS_CMDSTS_MORE)))
> #define SIS_OWNDESC(x)
(letoh32((x)->sis_ctl) & SIS_CMDSTS_OWN)
> #define SIS_INC(x, y) (x) =
((x) == ((y)-1)) ? 0 : (x)+1
> -#define SIS_RXBYTES(x)
> -
((letoh32((x)->sis_ctl) & SIS_CMDSTS_BUFLEN) - ETHER_CRC_LEN)
> +#define
SIS_RXBYTES(x) (letoh32((x)->sis_ctl) & SIS_CMDSTS_BUFLEN)
>
> #define
SIS_RXSTAT_COLL 0x0001
> #define SIS_RXSTAT_LOOPBK 0x0002
>
Index: if_sis.c
>
===
>
RCS file: /cvs/src/sys/dev/pci/if_sis.c,v
> retrieving revision 1.132
>
diff -u -p -r1.13
Don't override all errors in opendev
Not all errors coming from diskmap should be overwritten.
In particular, if we get EBUSY, that doesn't mean "try again with
accessing /dev/foo, fail and return ENOENT instead".
Maybe we should add more errors to this list, or even invert the
filtering logic - I'm: 1) not an expert in this area; 2) open
for improvements.
How to reproduce: tunefs /
Before patch:
tunefs: /dev/r286cb876606dc1e0.l: No such file or directory
After patch:
tunefs: /dev/sd0l: Device busy
Okay?
--
WBR,
Vadim Zhukov
Index: opendev.c
===
RCS file: /cvs/src/lib/libutil/opendev.c,v
retrieving revision 1.15
diff -u -p -r1.15 opendev.c
--- opendev.c 30 Jun 2011 15:04:58 - 1.15
+++ opendev.c 19 Jan 2016 22:05:12 -
@@ -79,7 +79,8 @@ opendev(const char *path, int oflags, in
if (ioctl(fd, DIOCMAP, &dm) == -1) {
close(fd);
fd = -1;
- errno = ENOENT;
+ if (errno != EBUSY)
+ errno = ENOENT;
}
}
}
Re: Don't override all errors in opendev
2016-01-20 1:10 GMT+03:00 Vadim Zhukov : > Not all errors coming from diskmap should be overwritten. > In particular, if we get EBUSY, that doesn't mean "try again with > accessing /dev/foo, fail and return ENOENT instead". > > Maybe we should add more errors to this list, or even invert the > filtering logic - I'm: 1) not an expert in this area; 2) open > for improvements. > > How to reproduce: tunefs / > > Before patch: > tunefs: /dev/r286cb876606dc1e0.l: No such file or directory This one should be ".a", not ".l", of course: I've messed up with output from different partition, sorry. > After patch: > tunefs: /dev/sd0l: Device busy -- WBR, Vadim Zhukov
Fix nfs_nget() description
Apparently the function was refactored and but the old description remained. I came across this a few weeks ago. Does my revision look good? Index: nfs/nfs_node.c === RCS file: /cvs/src/sys/nfs/nfs_node.c,v retrieving revision 1.62 diff -u -p -r1.62 nfs_node.c --- nfs/nfs_node.c 14 Mar 2015 03:38:52 - 1.62 +++ nfs/nfs_node.c 20 Jan 2016 01:56:19 - @@ -76,10 +76,9 @@ RB_PROTOTYPE(nfs_nodetree, nfsnode, n_en RB_GENERATE(nfs_nodetree, nfsnode, n_entry, nfsnode_cmp); /* - * Look up a vnode/nfsnode by file handle. + * Look up a vnode/nfsnode by file handle and store the pointer in *npp. * Callers must check for mount points!! - * In all cases, a pointer to a - * nfsnode structure is returned. + * An error number is returned. */ int nfs_nget(struct mount *mnt, nfsfh_t *fh, int fhsize, struct nfsnode **npp)
segfault with tail -f
First noticed while running (and interrupting) dpb whilst tailing logs. Sequence of events, it may be relevant that $dir is on NFS: - mkdir $dir/a - for i in 1 2 3; do echo foo > $dir/a/$i; done - tail -f $dir/a/* - (in another window) doas rm -rf $dir #0 0x1664d4fa in tfreopen (tf=0x7f57732c) at forward.c:353 353 if (stat(ttf->fname, &sb) == -1) (gdb) bt #0 0x1664d4fa in tfreopen (tf=0x7f57732c) at forward.c:353 #1 0x1664cca8 in forward (tf=0x7f577000, nfiles=10, style=RLINES, origoff=10) at forward.c:224 #2 0x1664f098 in main (argc=10, argv=0xcf7c96f8) at tail.c:167 (gdb) frame 0 #0 0x1664d4fa in tfreopen (tf=0x7f57732c) at forward.c:353 353 if (stat(ttf->fname, &sb) == -1) (gdb) p ttf $1 = (struct tailfile *) 0x0
I have a mirror testing program for you.
I have a 500 line program I wrote that reads openbsd.org.ftp.html and
scraps off the html and ftp mirrors, records them all without redundancies
as http mirrors in memory and downloads the appropriate version and machine
architecture's SHA256 in the package folder. It tests all the mirrors for
time, one at a time and uses kqueue to kill any laggy ftp calls. It uses
ftp() calls for all its networking, so it shouldn't be too much of a
security issue I'd guess. It writes the top 8 mirrors into /etc/pkg.conf it
erases all the installpath entries while leaving everything else in the
file. It can run as an unprivileged user, but of course it won't rewrite
/etc/pkg.conf
-Luke N Small
/*
* Copyright (c) 2016 Luke N. Small
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* Special thanks to Dan Mclaughlin for the ftp to sed idea
*
* ftp -o - http://www.openbsd.org/ftp.html | \
* sed -n \
* -e 's: \([^<]*\)<.*:\1 :p' \
* -e 's:^\( [hfr].*\):\1:p'
*/
#define EVENT_NOPOLL
#define EVENT_NOSELECT
#include
#include
#include
#include
#include
#include
#include
#include
struct mirror_st
{
char * countryTitle;
char * mirror;
char * installPath;
double diff;
struct mirror_st * next;
};
int ftp_cmp (const void * a, const void * b)
{
struct mirror_st **one = (struct mirror_st **)a;
struct mirror_st **two = (struct mirror_st **)b;
if( (*one)->diff < (*two)->diff )
return -1;
if( (*one)->diff > (*two)->diff )
return 1;
return 0;
}
int country_cmp (const void * a, const void * b)
{
struct mirror_st **one = (struct mirror_st **)a;
struct mirror_st **two = (struct mirror_st **)b;
// list the USA mirrors first, it will subsort correctly
int8_t temp = !strncmp("USA", (*one)->countryTitle, 3);
if(temp != !strncmp("USA", (*two)->countryTitle, 3))
{
if(temp)
return -1;
return 1;
}
return strcmp( (*one)->countryTitle, (*two)->countryTitle ) ;
}
double getTimeDiff(struct timeval a, struct timeval b)
{
long sec;
long usec;
sec = b.tv_sec - a.tv_sec;
usec = b.tv_usec - a.tv_usec;
if (usec < 0)
{
--sec;
usec += 100;
}
return sec + ((double)usec / 100.0);
}
int main()
{
pid_t ftpPid, sedPid;
int ftpToSed[2];
int sedToParent[2];
int unameToParent[2];
char unameR[5], unameM[20];
int i = 0, c;
register int position, num;
FILE *input;
pipe(unameToParent);
// "uname -rm" returns version and architecture like: "5.8 amd64" to standard out
if(fork() == (pid_t) 0)
{ /* uname child */
close(unameToParent[0]);
dup2(unameToParent[1], STDOUT_FILENO); /*attaching to pipe(s)*/
execl("/usr/bin/uname","/usr/bin/uname", "-rm", NULL);
err(1, "execl() failed\n");
}
close(unameToParent[1]);
input = fdopen (unameToParent[0], "r");
num = 0;
position = -1;
while ((c = getc(input)) != EOF)
{
if(num == 0)
{
if(c != ' ')
unameR[++position] = c;
else
{
unameR[position + 1] = '\0';
num = 1;
position = -1;
}
}
else
{
if(c != '\n')
unameM[++position] = c;
else
unameM[position + 1] = '\0';
}
}
fclose (input);
close(unameToParent[0]);
pipe(ftpToSed); /*make pipes*/
struct kevent ke[2];
int kq = kqueue();
if (kq == -1)
err(1, "kq!");
int kqProc = kqueue();
if (kqProc == -1)
err(1, "kqProc!");
ftpPid = fork();
if(ftpPid == (pid_t) 0)
{ /*ftp child*/
close(ftpToSed[0]);
dup2(ftpToSed[1], STDOUT_FILENO); /*attaching to pipe(s)*/
execl("/usr/bin/ftp","ftp", "-Vo", "-", "http://www.openbsd.org/ftp.html";, NULL);
err(1, "execl() failed\n");
}
EV_SET(ke, ftpPid, EVFILT_PROC, EV_ADD | EV_ONESHOT, NOTE_EXIT, 0, &ftpPid);
if (kevent(kqProc, ke, 1, NULL, 0, NULL) == -1)
err(1, "kevent register fail.");
close(ftpToSed[1]);
pipe(sedToParent);
sedPid = fork();
if(sedPid == (pid_t) 0)
{ /* sed child */
close(sedToParent[0]);
dup2(ftpToSed[0], STDIN_FILENO); /*attaching to pipe(s)*/
dup2(sedToParent[1], STDOUT_FILENO);
execl("/usr/bin/sed","sed","-n","-e", "s:\t\\([^<]*\\)<.*:\\1 :p",
"-e", "s:^\\(\t[hfr].*\\):\\1:p", NULL);
kill(ftpPid, SIGKILL);
err(1, "execl() failed\n");
}
EV_SET(ke, sedPid, EVFILT_PROC, EV_ADD | EV_ONESHOT, NOTE_EXIT, 0, &sedPid);
if (kevent(kqPro
Quoting ${CC} expansion in libiberty Makefile
Previously sent to misc@, but I was told to send it here instead.
Cheers,
Andreas
- Forwarded message from Andreas Kusalananda Kähäri
-
Date: Mon, 18 Jan 2016 16:33:34 +0100
From: Andreas Kusalananda Kähäri
To: openbsd-misc
Subject: Quoting ${CC} expansion in libiberty Makefile
Hi,
I tried running the base system build with CC="ccache cc" and it broke
while compiling libiberty due to an unquoted expansion of ${CC} in the
Makefile ("${MAKE} ${GNUCFLAGS} CC=${CC} needed-list").
I know I won't save much time by using ccache since the compiler is
rebuilt during the process, but I assume that a similar problem would
arise if one tried to use CC="distcc cc", for example. Also, other
Makefiles in the tree quotes ${CC}, and this is *almost* the sole
instance where it's not quoted. Other instances are
lib/libcrypto/crypto/arch/{mips64,sparc64}/Makefile.inc
Patch:
Index: Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/lib/libiberty/Makefile.bsd-wrapper,v
retrieving revision 1.15
diff -u -p -u -r1.15 Makefile.bsd-wrapper
--- Makefile.bsd-wrapper31 Aug 2014 01:02:48 - 1.15
+++ Makefile.bsd-wrapper18 Jan 2016 15:28:44 -
@@ -32,7 +32,7 @@ CLEANFILES+= Makefile config.cache confi
depend:needed-list
needed-list: config.status
- ${MAKE} ${GNUCFLAGS} CC=${CC} needed-list
+ ${MAKE} ${GNUCFLAGS} CC="${CC}" needed-list
config.status: Makefile.in configure
PATH="/bin:/usr/bin:/sbin:/usr/sbin" \
--
Andreas Kusalananda Kähäri, Bioinformatics Developer, Uppsala, Sweden
OpenPGP: url=https://db.tt/2zaB1E7y; id=46082BDF
- End forwarded message -
--
Andreas Kusalananda Kähäri, Bioinformatics Developer, Uppsala, Sweden
OpenPGP: url=https://db.tt/2zaB1E7y; id=46082BDF
signature.asc
Description: PGP signature
