Re: Odd Public WiFi breaks dhclient(8) but works for iPhone (Fix!)
On Sun, Jun 10, 2018 at 02:15:09PM -0400, Kenneth R Westerback wrote:
> On Sat, Jun 09, 2018 at 02:10:09PM +0200, Claudio Jeker wrote:
> > On Sat, Jun 09, 2018 at 01:31:20PM +0200, Martin Pieuchot wrote:
> > > On 08/06/18(Fri) 18:06, Kenneth R Westerback wrote:
> > > > Testing at the alternate DHCP lab (the one that serves beer) I find
> > > > that its wifi gives me the lease
> > > >
> > > > lease {
> > > > fixed-address 10.112.38.73;
> > > > next-server 0.0.0.0;
> > > > option subnet-mask 255.255.255.0;
> > > > option routers 10.112.33.1;
> > > > option domain-name-servers 63.250.111.34,63.250.111.35,8.8.8.8;
> > > > option dhcp-lease-time 14400;
> > > > option dhcp-message-type 5;
> > > > option dhcp-server-identifier 10.112.38.1;
> > > > option dhcp-renewal-time 7200;
> > > > option dhcp-rebinding-time 12600;
> > > > option dhcp-client-identifier 1:9c:4e:36:d6:7e:f8;
> > > > epoch 1528494503;
> > > > renew 5 2018/06/08 23:48:23 UTC;
> > > > rebind 6 2018/06/09 01:18:23 UTC;
> > > > expire 6 2018/06/09 01:48:23 UTC;
> > > > }
> > > >
> > > > See the problem? If so, skip the next paragraph.
> > > >
> > > > Note that I get an address of 10.112.38.73/24 and a default route of
> > > > 10.112.33.1. When dhclient attempts to add the default route our stack
> > > > rejects the attempt as 10.112.33.1 is unreachable! Not only does this
> > > > mean the outside world is unreachable, /etc/resolv.conf will not be
> > > > updated because the wifi interface will not own the default route, and
> > > > thus DNS will not work!
> > > >
> > > > As this particular DHCP testing facility not only serves beer but
> > > > provides paper table coverings and a basket of crayons (although I had
> > > > to surrender my blue crayon to a young artist at the next table) I
> > > > was able to do some complex bit calculations and determine that
> > > > 255.255.240.0 would put the default route and the address in the same
> > > > subnet. I therefore added
> > > >
> > > > supersede subnet-mask 255.255.240.0;
> > > >
> > > > to my dhclient.conf and viola (sic)! I had net.
> > > >
> > > > Checking the iPhone I see the same issue, but the iPhone does connect
> > > > to the network without manual magic. A little birdie told me that IOS
> > > > may be playing games with the provided subnet mask to ensure the
> > > > default route is reachable.
> > >
> > > Can you check what is the configured address' mask on iOS?
> > >
> > > > I'm wondering if this auto subnet-mask trimming to ensure the default
> > > > route would be reachable is worthwhile adding? Or if it might break
> > > > more situations than it fixes.
> > >
> > > I'd say that there's nothing more frustrating than having a non functional
> > > network connection after having used dhclient(8). So I doubt we're going
> > > to break anything. However I'm wondering what other OSes are doing
> > > because
> > > I'm not sure we should work around broken configs :)
> >
> > Isn't this similar to the google cloud dhcp mode where you get a /32 host
> > IP and a gateway (which is not part of the the /32 obviously).
> > IIRC this is what some other systems do more or less.
> > I think a trick could be to insert the gateway as a /32 cloning route then
> > arp would resolve the gateway which I assume works just fine. Now how to
> > do this exactly is an excercise for the reader ;)
> >
> >
> > --
> > :wq Claudio
>
> Turning Claudio's idea into a diff gives the diff below.
>
> Testing at DHCP Lab B gives a routing table
>
> Internet:
> DestinationGatewayFlags Refs Use Mtu Prio Iface
> Label
> default10.112.33.1GS29 187 -12 iwn0
> 224/4 127.0.0.1 URS0 23 32768 8 lo0
> 10.112.33/24 10.112.38.215 CS 10 -12 iwn0
> 10.112.33.1a4:6c:2a:5e:8a:de HLch 15 -11 iwn0
> 10.112.38/24 10.112.38.215 Cn 09 - 8 iwn0
> 10.112.38.215 9c:4e:36:d6:7e:f8 UHLl 0 35 - 1 iwn0
> 10.112.38.255 10.112.38.215 Hb 01 - 1 iwn0
> 127/8
Odd Public WiFi breaks dhclient(8) but works for iPhone (Fix!)
On Sat, Jun 09, 2018 at 02:10:09PM +0200, Claudio Jeker wrote:
> On Sat, Jun 09, 2018 at 01:31:20PM +0200, Martin Pieuchot wrote:
> > On 08/06/18(Fri) 18:06, Kenneth R Westerback wrote:
> > > Testing at the alternate DHCP lab (the one that serves beer) I find
> > > that its wifi gives me the lease
> > >
> > > lease {
> > > fixed-address 10.112.38.73;
> > > next-server 0.0.0.0;
> > > option subnet-mask 255.255.255.0;
> > > option routers 10.112.33.1;
> > > option domain-name-servers 63.250.111.34,63.250.111.35,8.8.8.8;
> > > option dhcp-lease-time 14400;
> > > option dhcp-message-type 5;
> > > option dhcp-server-identifier 10.112.38.1;
> > > option dhcp-renewal-time 7200;
> > > option dhcp-rebinding-time 12600;
> > > option dhcp-client-identifier 1:9c:4e:36:d6:7e:f8;
> > > epoch 1528494503;
> > > renew 5 2018/06/08 23:48:23 UTC;
> > > rebind 6 2018/06/09 01:18:23 UTC;
> > > expire 6 2018/06/09 01:48:23 UTC;
> > > }
> > >
> > > See the problem? If so, skip the next paragraph.
> > >
> > > Note that I get an address of 10.112.38.73/24 and a default route of
> > > 10.112.33.1. When dhclient attempts to add the default route our stack
> > > rejects the attempt as 10.112.33.1 is unreachable! Not only does this
> > > mean the outside world is unreachable, /etc/resolv.conf will not be
> > > updated because the wifi interface will not own the default route, and
> > > thus DNS will not work!
> > >
> > > As this particular DHCP testing facility not only serves beer but
> > > provides paper table coverings and a basket of crayons (although I had
> > > to surrender my blue crayon to a young artist at the next table) I
> > > was able to do some complex bit calculations and determine that
> > > 255.255.240.0 would put the default route and the address in the same
> > > subnet. I therefore added
> > >
> > > supersede subnet-mask 255.255.240.0;
> > >
> > > to my dhclient.conf and viola (sic)! I had net.
> > >
> > > Checking the iPhone I see the same issue, but the iPhone does connect
> > > to the network without manual magic. A little birdie told me that IOS
> > > may be playing games with the provided subnet mask to ensure the
> > > default route is reachable.
> >
> > Can you check what is the configured address' mask on iOS?
> >
> > > I'm wondering if this auto subnet-mask trimming to ensure the default
> > > route would be reachable is worthwhile adding? Or if it might break
> > > more situations than it fixes.
> >
> > I'd say that there's nothing more frustrating than having a non functional
> > network connection after having used dhclient(8). So I doubt we're going
> > to break anything. However I'm wondering what other OSes are doing because
> > I'm not sure we should work around broken configs :)
>
> Isn't this similar to the google cloud dhcp mode where you get a /32 host
> IP and a gateway (which is not part of the the /32 obviously).
> IIRC this is what some other systems do more or less.
> I think a trick could be to insert the gateway as a /32 cloning route then
> arp would resolve the gateway which I assume works just fine. Now how to
> do this exactly is an excercise for the reader ;)
>
>
> --
> :wq Claudio
Turning Claudio's idea into a diff gives the diff below.
Testing at DHCP Lab B gives a routing table
Internet:
DestinationGatewayFlags Refs Use Mtu Prio Iface
Label
default10.112.33.1GS29 187 -12 iwn0
224/4 127.0.0.1 URS0 23 32768 8 lo0
10.112.33/24 10.112.38.215 CS 10 -12 iwn0
10.112.33.1a4:6c:2a:5e:8a:de HLch 15 -11 iwn0
10.112.38/24 10.112.38.215 Cn 09 - 8 iwn0
10.112.38.215 9c:4e:36:d6:7e:f8 UHLl 0 35 - 1 iwn0
10.112.38.255 10.112.38.215 Hb 01 - 1 iwn0
127/8 127.0.0.1 UGRS 00 32768 8 lo0
127.0.0.1 127.0.0.1 UHhl 12 32768 1 lo0
x230$
and a resolv.conf
# Generated by iwn0 dhclient
nameserver 63.250.111.34
nameserver 63.250.111.35
nameserver 8.8.8.8
lookup file bind
family inet4
after getting the lease
lease {
fixed-address 10.112.38.215;
next-server 0.0.0.0;
option subnet-mask 255.255.255.0;
option routers 10.112.33.1;
option domain-name-servers 63.250.111.34,6
Re: dhcp-options(5) diff
On Wed, Feb 28, 2018 at 05:27:41PM +0100, Matthieu Herrb wrote: > On Wed, Feb 28, 2018 at 05:24:20PM +0100, Matthieu Herrb wrote: > > Hi, > > > > I've started using the classless-static-route option in dhcpd(8). This > > was not as painless as possible because I missed some important > > information from the underlying RFC to understand how the option is > > used by clients and how it should be configured on the server. > > > > The patch below tries to add this information to dhcp-options(5), > > without beeing too verbose. > > > > While there, I also noted that the classful routes are obsolete. I was > > wondering if we could just remove this option from the doc (and maybe > > tedu the code ?) instead. > > > > Ok, corrections, ... ? > > Argh I managed to mangle the diff with a last minute edit. Correct > version: I agree with the intent of this diff and am happy to work on tweaks to the actual verbiage in-tree. ok krw@ Ken > > Index: dhcp-options.5 > === > RCS file: /cvs/OpenBSD/src/usr.sbin/dhcpd/dhcp-options.5,v > retrieving revision 1.22 > diff -u -p -u -r1.22 dhcp-options.5 > --- dhcp-options.514 Sep 2015 20:06:59 - 1.22 > +++ dhcp-options.528 Feb 2018 16:26:50 - > @@ -171,6 +171,11 @@ RFC 1122. > .It Ic option classless-static-routes Ar ip/prefix ip Oo , Ar ip/prefix ip > ... Oc ; > This option specifies a list of static routes in CDIR notation, which > should be sent to the client. > +This option is specified in RFC3442. > +Note that, since the RFC says that clients supporting this option must > ignore the > +.Ic Routers > +option when both are present, the default route > +needs to be included in the list of routes specified here. > .It Ic option classless-ms-static-routes Ar ip/prefix ip Oo , Ar ip/prefix > ip ... Oc ; > This option does the same as > .Ic classless-static-routes , > @@ -517,6 +522,8 @@ The default route (0.0.0.0) is an illega > To specify the default route, use the > .Ic routers > option. > +Note that this option is obsolete and should be replaced by the > +.Ic classless-static-routes option. > .It Ic option streettalk-directory-assistance-server Ar ip-address Oo , Ar > ip-address ... Oc ; > The StreetTalk Directory Assistance (STDA) server option specifies a > list of STDA servers available to the client. > > -- > Matthieu Herrb
Re: dhcpd: don't reject DHCPINFORM from behind relay
On Wed, Jul 05, 2017 at 04:37:39PM +0200, Reyk Floeter wrote:
> Hi,
>
> landry@ sees many log messages 'DHCPINFORM from xx but ciaddr yy is
> not consistent with actual address' in a setup where dhcpd runs behind
> dhcrelay.
>
> The code in dhcpd's dhcpinform() seems wrong - it assumes that ciaddr
> (the client IP) is identical to the packet source address and it
> doesn't consider a relay, indicated by giaddr (gateway).
>
> I looked at isc-dhcpd code and, omg my eyes are bleeding, it seems to
> handle DHCPINFORM from relayed clients with giaddr set.
>
> So it seems that dhcpd never accepted DHCPINFORM from clients behind a
> relay, and the diff below changes it and stops the log spam. But it
> also changes behavior, so it needs some testing and maybe feedback
> from DHCP experts (prodding krw).
>
> Comments?
Can' find anything that says this would be wrong. I have no way to
test.
Ken
>
> Reyk
>
> Index: usr.sbin/dhcpd/dhcp.c
> ===
> RCS file: /cvs/src/usr.sbin/dhcpd/dhcp.c,v
> retrieving revision 1.56
> diff -u -p -u -1 -1 -p -r1.56 dhcp.c
> --- usr.sbin/dhcpd/dhcp.c 24 Apr 2017 14:58:36 - 1.56
> +++ usr.sbin/dhcpd/dhcp.c 5 Jul 2017 14:23:12 -
> @@ -519,23 +519,23 @@ void
> dhcpinform(struct packet *packet)
> {
> struct lease lease;
> struct iaddr cip;
> struct subnet *subnet;
>
> /*
>* ciaddr should be set to client's IP address but
>* not all clients are standards compliant.
>*/
> cip.len = 4;
> - if (packet->raw->ciaddr.s_addr) {
> + if (packet->raw->ciaddr.s_addr && !packet->raw->giaddr.s_addr) {
> if (memcmp(>raw->ciaddr.s_addr,
> packet->client_addr.iabuf, 4) != 0) {
> log_info("DHCPINFORM from %s but ciaddr %s is not "
> "consistent with actual address",
> piaddr(packet->client_addr),
> inet_ntoa(packet->raw->ciaddr));
> return;
> }
> memcpy(cip.iabuf, >raw->ciaddr.s_addr, 4);
> } else
> memcpy(cip.iabuf, >client_addr.iabuf, 4);
>
softraid and 4096-byte sectors 'fixed'
The diff below is a first cut at making softraid usable on today's
larger and larger disks which use 4096-byte sectors.
It allows building softraid volumes with such devices, and even
building volumes that mix 'classic' 512-byte sector devices with
'avante garde' 4k-sector devices.
Unlikely to be completely final, but lots of testing would be
appreciated. And I mean testing -current with existing 512-byte
sector softraid volumes! A significant amount of cleanup was committed
to prepare for this diff. If you want to ensure a surprise-free
upgrade to 5.8, now is the time to test.
NEEDED: a machine with a BIOS that can boot from a 4K device.
Apparently very rare still. But it would be nice to test with.
CAVEAT: The metadata version has changed so new volumes you create
will not be loadable on boxes running older versions of OpenBSD.
CAVEAT: You can't rebuild a volume created with *only* 512-byte
devices onto a 4K-sector device. The volume must be created with
at least one 4K device to start with.
Ken
Index: softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.360
diff -u -p -r1.360 softraid.c
--- softraid.c 21 Jul 2015 03:30:51 - 1.360
+++ softraid.c 21 Jul 2015 04:02:14 -
@@ -944,6 +944,7 @@ sr_meta_validate(struct sr_discipline *s
*/
if (sm-ssd_data_blkno == 0)
sm-ssd_data_blkno = SR_META_V3_DATA_OFFSET;
+ sm-ssdi.ssd_secsize = DEV_BSIZE;
} else if (sm-ssdi.ssd_version == 4) {
@@ -953,14 +954,22 @@ sr_meta_validate(struct sr_discipline *s
*/
if (sm-ssd_data_blkno == 0)
sm-ssd_data_blkno = SR_DATA_OFFSET;
+ sm-ssdi.ssd_secsize = DEV_BSIZE;
- } else if (sm-ssdi.ssd_version == SR_META_VERSION) {
+ } else if (sm-ssdi.ssd_version == 5) {
/*
* Version 5 - variable length optional metadata. Migration
* from earlier fixed length optional metadata is handled
* in sr_meta_read().
*/
+ sm-ssdi.ssd_secsize = DEV_BSIZE;
+
+ } else if (sm-ssdi.ssd_version == SR_META_VERSION) {
+
+ /*
+* Version 6 - store report a sector size.
+*/
} else {
@@ -1052,13 +1061,6 @@ sr_meta_native_bootprobe(struct sr_softc
}
vput(vn);
- /* Make sure this is a DEV_BSIZE byte/sector device. */
- if (label.d_secsize != DEV_BSIZE) {
- DNPRINTF(SR_D_META, %s: %s has unsupported sector size (%d),
- DEVNAME(sc), devname, label.d_secsize);
- goto done;
- }
-
md = malloc(SR_META_SIZE * DEV_BSIZE, M_DEVBUF, M_ZERO | M_NOWAIT);
if (md == NULL) {
sr_error(sc, not enough memory for metadata buffer);
@@ -1568,13 +1570,6 @@ sr_meta_native_probe(struct sr_softc *sc
}
memcpy(ch_entry-src_duid, label.d_uid, sizeof(ch_entry-src_duid));
- /* Make sure this is a DEV_BSIZE byte/sector device. */
- if (label.d_secsize != DEV_BSIZE) {
- sr_error(sc, %s has unsupported sector size (%u),
- devname, label.d_secsize);
- goto unwind;
- }
-
/* make sure the partition is of the right type */
if (label.d_partitions[part].p_fstype != FS_RAID) {
DNPRINTF(SR_D_META,
@@ -1597,6 +1592,7 @@ sr_meta_native_probe(struct sr_softc *sc
goto unwind;
}
ch_entry-src_size = size;
+ ch_entry-src_secsize = label.d_secsize;
DNPRINTF(SR_D_META, %s: probe found %s size %lld\n, DEVNAME(sc),
devname, (long long)size);
@@ -2879,11 +2875,6 @@ sr_hotspare(struct sr_softc *sc, dev_t d
vput(vn);
goto fail;
}
- if (label.d_secsize != DEV_BSIZE) {
- sr_error(sc, %s has unsupported sector size (%u),
- devname, label.d_secsize);
- goto fail;
- }
if (label.d_partitions[part].p_fstype != FS_RAID) {
sr_error(sc, %s partition not of type RAID (%d),
devname, label.d_partitions[part].p_fstype);
@@ -2943,6 +2934,7 @@ sr_hotspare(struct sr_softc *sc, dev_t d
sm-ssdi.ssd_volid = SR_HOTSPARE_VOLID;
sm-ssdi.ssd_level = SR_HOTSPARE_LEVEL;
sm-ssdi.ssd_size = size;
+ sm-ssdi.ssd_secsize = label.d_secsize;
strlcpy(sm-ssdi.ssd_vendor, OPENBSD, sizeof(sm-ssdi.ssd_vendor));
snprintf(sm-ssdi.ssd_product, sizeof(sm-ssdi.ssd_product),
SR %s, HOTSPARE);
@@ -3193,11 +3185,6 @@ sr_rebuild_init(struct sr_discipline *sd
DEVNAME(sc));
goto done;
}
- if (label.d_secsize != DEV_BSIZE) {
- sr_error(sc, %s has unsupported sector size (%u),
-
Microsoft Now OpenBSD Foundation Gold Contributor
The OpenBSD Foundation is happy to announce that Microsoft has made a significant financial donation to the Foundation. This donation is in recognition of the role of the Foundation in supporting the OpenSSH project. This donation makes Microsoft the first Gold level contributor in the OpenBSD Foundation's 2015 fundraising campaign. Donations to the Foundation can be made on our Donations Page at www.openbsdfoundation.org/donations.html We can be contacted regarding corporate sponsorship at fundrais...@openbsdfoundation.org.
OpenBSD Foundation 2014/2015 News Fundraising
2014 was the most successful year to date for the OpenBSD Foundation. Both in the amount of money we raised and in the support we provided for the OpenBSD and related projects. We are extremely grateful for the support shown by our contributers large and small. A detailed summary of the Foundation's activities in 2014 can be seen at http://www.openbsdfoundation.org/activities.html But here are some highpoints. We received $397,000 in new donations and paid out $129,000 to support the activities of the OpenBSD and related projects. Some of the things the $129,000 made happen were higher speed network links to the project's machine room; new servers, UPSs, network switches, serial consoles and network monitoring equipment for the machine room; development machines for several developers; participation in GSOC 2014; and hackathons in Lujbljana, Dunedin, Berlin, and Marrakesh. As you can see, 2014 was a very good year for the Foundation. This can be attributed to a number of unique events. A very public finanical crisis at the start of the year generated extensive community support, and the releases of LibreSSL generated significant interest and support. But it is important for us not to rely on one time events for our success. Which brings us to our 2015 Fundraising Campaign, described at http://www.openbsdfoundation.org/campaign2015.html The OpenBSD Foundation needs your help to achieve our fundraising goal of $200,000 for 2015. We need both Individual and Corporate sponsorship of the Foundation. Reaching this goal will ensure the continued health of the projects we support, will enable us to help them do more, and will avoid the distraction of financial emergencies that could spell the end of the projects. In particular it would allow us to fund more dedicated developer time for targeted development of specific projects. If $10 were given for every installation of OpenBSD in the last year from the master site we would be at our goal. If $2 were given for every download of the OpenSSH source code in the last year from the master site we would be at our goal. If a penny was donated for every pf or OpenSSH installed with a mainstream operating system or phone in the last year we would be at our goal. As an individual or corporation, the best kind of donation we can receive is a recurring donation. This allows longer term planning on our part, instead of hoping for one time cash infusions. The easiest way for an individual to support us in this way is a recurring Paypal donation, which is our preference. Donations to the foundation can be made on our Donations Page. http://www.openbsdfoundation.org/donations.html We can be contacted regarding corporate sponsorship at fundrais...@openbsdfoundation.org
growfs fix
This seems to fix growfs on 4k-sector drives by doing the test write
to the last sector rather than the last 512-byte block, which can't be
accessed directly on 4k-sector drives.
Any other growfs users out there want to test on 'normal' drives?
Ken
Index: growfs.c
===
RCS file: /cvs/src/sbin/growfs/growfs.c,v
retrieving revision 1.33
diff -u -p -r1.33 growfs.c
--- growfs.c10 Nov 2013 00:48:04 - 1.33
+++ growfs.c29 Apr 2014 15:42:56 -
@@ -1899,7 +1899,7 @@ int
main(int argc, char **argv)
{
DBG_FUNC(main)
- char*device;
+ char*device, *lastsector;
int ch;
long long size = 0;
unsigned intNflag = 0;
@@ -2072,12 +2072,16 @@ main(int argc, char **argv)
(intmax_t)sblock.fs_size);
/*
-* Try to access our new last block in the filesystem. Even if we
-* later on realize we have to abort our operation, on that block
+* Try to access our new last sector in the filesystem. Even if we
+* later on realize we have to abort our operation, on that sector
* there should be no data, so we can't destroy something yet.
*/
- wtfs(DL_SECTOBLK(lp, DL_GETPSIZE(pp)) - 1, (size_t)DEV_BSIZE,
- (void *)sblock, fso, Nflag);
+ lastsector = calloc(1, lp-d_secsize);
+ if (!lastsector)
+ err(1, No memory for last sector test write);
+ wtfs(DL_SECTOBLK(lp, DL_GETPSIZE(pp) - 1), lp-d_secsize,
+ lastsector, fso, Nflag);
+ free(lastsector);
/*
* Now calculate new superblock values and check for reasonable
Re: 5.4 amd64 - Poor disk performance with Smart Array 6404
On Mon, Dec 09, 2013 at 07:24:19PM -0500, Adam Jensen wrote:
I recently (last night) installed OpenBSD-5.4-amd64 on an
HP-Proliant ML370-G4 that has a Smart Array 6404 controller card in
a 64-bit, 133-MHz PCI-X slot. It has two Ultra320 SCSI channels and
192MB of RAM cache. One SCSI channel is connected to two 146GB U320
10kRPM drives which are configured as a RAID0 array. The Smart Array
card presents the RAID0 as one SCSI device: /dev/sd0.
[dmesg]: http://pastebin.com/Sxs801ef
[pcidump]: http://pastebin.com/P5Zn1xM4
[sysctl]: http://pastebin.com/kmXeMZ02
[acpidump]: http://pastebin.com/xufZXdha acpi.headers only
Disk performance is *very* bad. For example:
dd if=/dev/zero of=test bs=1k count=32768
32768+0 records in
32768+0 records out
33554432 bytes transferred in 6.325 secs (5304659 bytes/sec)
I tinkered with FreeBSD9.2 and CentOS6.4 on this hardware and they
both transfer about 300M to 400M bytes/sec with that command. Large
transfers (GB) write about 80M to 100M bytes/sec (if I remember
correctly).
I am preparing to recompile the system to 5.4-stable so there is an
opportunity to make some tweaks to the default kernel configuration
(maybe the SCSIDEBUG option) but I am very open to suggestions on
how to proceed.
I plan to add four more 146GB U320 10kRPM drives, place them on the
second Ultra320 SCSI channel, and configure them as a RAID5 array. I
intend to run OpenBSD on this machine for the foreseeable future so
getting the RAID hardware configured for maximum utilization,
performance, and reliability is of utmost importance. Any RAID
hardware gurus willing to point me in the right direction will be
much appreciated. Actually, *any* suggestions could potentially be
useful. I'm rather stuck at the moment.
Hmm. Googling 'openbsd ciss slow' led me to
http://openbsd.7691.n7.nabble.com/ciss-4-write-very-slow-w-o-bbwc-td93173.html
which led me to
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ic/ciss.c.diff?r1=1.22r2=1.23only_with_tag=MAINf=h
which induced me to create the diff below. This may not be the correct
or final solution, but it would be interesting to see if it changes the
performance on your machine.
Ken
Index: ciss.c
===
RCS file: /cvs/src/sys/dev/ic/ciss.c,v
retrieving revision 1.68
diff -u -p -r1.68 ciss.c
--- ciss.c 30 May 2013 16:15:02 - 1.68
+++ ciss.c 10 Dec 2013 00:52:51 -
@@ -607,6 +607,7 @@ ciss_cmd(struct ciss_ccb *ccb, int flags
int
ciss_done(struct ciss_ccb *ccb)
{
+ struct scsi_inquiry_data *inq;
struct ciss_softc *sc = ccb-ccb_sc;
struct scsi_xfer *xs = ccb-ccb_xs;
struct ciss_cmd *cmd = ccb-ccb_cmd;
@@ -635,6 +636,13 @@ ciss_done(struct ciss_ccb *ccb)
if (xs) {
xs-resid = 0;
+ if (xs-cmd-opcode == INQUIRY error != 0) {
+ /* Adaptor doesn't bother to claim being SCSI2+. */
+ inq = (struct scsi_inquiry_data *)xs-data;
+ if (SCSISPC(inq-version) == 0
+ (inq-flags SID_CmdQue) != 0)
+ inq-version |= 2;
+ }
scsi_done(xs);
}
Re: dhclient support for /32 assignments
On Wed, Dec 04, 2013 at 12:47:19PM -0800, Matthew Dempsky wrote: On Wed, Dec 04, 2013 at 02:10:21PM -0500, Kenneth R Westerback wrote: No, that was my point. i.e. don't avoid adding the route when given a /32 address just because class static routes are also present. I think there might be a misunderstanding, so let me back up and try to clarify. :) Compute Engine gives out DHCP leases like ip: 10.240.77.88, netmask: 255.255.255.255, gateway: 10.240.0.1. The problem is 10.240.0.1 isn't routable given a 10.240.77.88/32 IP address. Ah. That was *not* what I understood you were saying, so there was indeed a misunderstanding. :-) ISC DHCP on Linux handles this by special casing netmask == 255.255.255.255, and adding an extra direct route for the gateway IP. E.g., for the above assignment, ISC DHCP would run route add 10.240.0.1 dev eth0 which tells Linux's network stack 10.240.0.1 is directly accessible via eth0, even though it's not part of the 10.240.77.88/32 subnet. (It would then run route add default 10.240.0.1 as per normal.) This seems weird and wrong to me, but I hasten to add I don't mean that as an informed opinion of what should be done. It's just that what routing foo I have does not encompass routes via interfaces that don't have an address/mask that allow access to that ip address. Like I said, weird. As far as I can tell, there's no authoritative/standard document that describes this special case, and ISC DHCP only applies this special case to the default gateway IP specified by DHO_ROUTERS. Notably, it does *not* apply this direct routing logic for gateway IPs specified by DHO_STATIC_ROUTES and/or DHO_CLASSLESS_STATIC_ROUTES, but it's unclear whether that's intentional or accidental. Not from me. This is way bigger that the last three line diff, and I have insufficient routing foo to comment on the usefulness of all these routes. The original three line diff was so short because it mimicked ISC DHCP and only special cased the default gateway (i.e., DHO_ROUTERS), and only did so when DHO_ROUTERS was actually used (i.e., when DHO_CLASSLESS_STATIC_ROUTES is not specified). I was incorrectly reading your desire as wanting to add a route to the address being bound, not a route to the gateway. I also didn't understand why you wanted to do *that*, but it seemed a small, contained change. All the added complexity to attempt to address my misunderstanding was not needed. Sorry about the red herring! You suggested we should unconditionally add the route, but it was unclear to me whether you meant: 1. We should always apply the special case logic for adding a direct route for the default gateway specified by DHO_ROUTERS, even if DHO_CLASSLESS_STATIC_ROUTES is specified; or 2. We should apply the special case logic to add direct routes for all gateways (i.e., those specified by DHO_CLASSLESS_STATIC_ROUTES and DHO_STATIC_ROUTES too), not just the default gateway. Interpretation #1 didn't seem right, because if DHO_CLASSLESS_STATIC_ROUTES is specified, then DHO_ROUTERS isn't used at all, so it might not be meaningful to add a direct route for DHO_ROUTERS's gateway. Interpretation #2 seems reasonable (though deviating from ISC DHCP's undocumented behavior), so that's what the second patch implemented. But that's necessarily more code than the original diff, because we need to repeat the special case logic for each gateway IP as they're extracted from the DHCP options. The change is somewhat bigger and more invasive, but I think not that much more complex: 1. Add a function ensure_direct_route() function that applies ISC DHCP's /32 logic for a given gateway IP address (same code as from first patch). 2. Call ensure_direct_route() in each of add_default_route(), add_static_routes(), and add_classless_static_routes() for each gateway IP address. 3. Add extra 'addr' and 'addrmask' parameters to each of the add_*_routes() functions to pass the extra data needed for ensure_direct_route(), and fix call sites appropriately. 4. In add_static_routes(), rename the local 'addr' variable that points into option data to 'data' to avoid conflicting with the new 'addr' parameter. Given the above, I think the original change makes the most sense. Ken
Re: dhclient support for /32 assignments
On Wed, Dec 04, 2013 at 10:57:41AM -0800, Matthew Dempsky wrote:
On Tue, Dec 03, 2013 at 11:48:05PM -0500, Kenneth Westerback wrote:
Rfc 3442 is what I referred to.
I don't think RFC 3442 discusses what to do with /32 IP address
assignments though?
No, that was my point. i.e. don't avoid adding the route when given
a /32 address just because class static routes are also present.
Anyway, below is a revised diff that does the same direct-route magic
for all gateway IPs, not just the default gateway IP. I'll try this
out later today to make sure it still works with Compute Engine.
Assuming it works as intended, ok?
Not from me. This is way bigger that the last three line diff, and
I have insufficient routing foo to comment on the usefulness of
all these routes.
Ken
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.268
diff -u -p -r1.268 dhclient.c
--- dhclient.c20 Nov 2013 17:22:46 - 1.268
+++ dhclient.c4 Dec 2013 18:45:21 -
@@ -107,9 +107,11 @@ struct client_lease *clone_lease(struct
void socket_nonblockmode(int);
void apply_ignore_list(char *);
-void add_default_route(int, struct in_addr, struct in_addr);
-void add_static_routes(int, struct option_data *);
-void add_classless_static_routes(int, struct option_data *);
+void add_default_route(int, struct in_addr, struct in_addr, struct in_addr);
+void add_static_routes(int, struct in_addr, struct in_addr,
+struct option_data *);
+void add_classless_static_routes(int, struct in_addr, struct in_addr,
+struct option_data *);
int compare_lease(struct client_lease *, struct client_lease *);
@@ -871,8 +873,8 @@ bind_lease(void)
*/
add_address(ifi-name, ifi-rdomain, client-new-address, mask);
if (options[DHO_CLASSLESS_STATIC_ROUTES].len) {
- add_classless_static_routes(ifi-rdomain,
- options[DHO_CLASSLESS_STATIC_ROUTES]);
+ add_classless_static_routes(ifi-rdomain, client-new-address,
+ mask, options[DHO_CLASSLESS_STATIC_ROUTES]);
} else {
if (options[DHO_ROUTERS].len) {
memset(gateway, 0, sizeof(gateway));
@@ -880,11 +882,11 @@ bind_lease(void)
memcpy(gateway.s_addr, options[DHO_ROUTERS].data,
options[DHO_ROUTERS].len);
add_default_route(ifi-rdomain, client-new-address,
- gateway);
+ mask, gateway);
}
if (options[DHO_STATIC_ROUTES].len)
- add_static_routes(ifi-rdomain,
- options[DHO_STATIC_ROUTES]);
+ add_static_routes(ifi-rdomain, client-new-address,
+ mask, options[DHO_STATIC_ROUTES]);
}
client-new-resolv_conf = resolv_conf_contents(
@@ -2379,6 +2381,23 @@ priv_write_file(struct imsg_write_file *
}
/*
+ * If we were given a /32 IP address assignment, then make sure the
+ * gateway address is routable with the equivalent of
+ *
+ * route add -net $gw -netmask 255.255.255.255 -cloning -iface $addr
+ */
+void
+ensure_direct_route(int rdomain, struct in_addr addr, struct in_addr
addrmask,
+struct in_addr gateway)
+{
+ if (addrmask.s_addr == INADDR_BROADCAST) {
+ add_route(rdomain, gateway, addrmask, addr,
+ RTA_DST | RTA_NETMASK | RTA_GATEWAY,
+ RTF_CLONING | RTF_STATIC);
+ }
+}
+
+/*
* add_default_route is the equivalent of
*
* route -q $rdomain add default -iface $router
@@ -2388,11 +2407,14 @@ priv_write_file(struct imsg_write_file *
* route -q $rdomain add default $router
*/
void
-add_default_route(int rdomain, struct in_addr addr, struct in_addr gateway)
+add_default_route(int rdomain, struct in_addr addr, struct in_addr addrmask,
+struct in_addr gateway)
{
struct in_addr netmask, dest;
int addrs, flags;
+ ensure_direct_route(rdomain, addr, addrmask, gateway);
+
memset(netmask, 0, sizeof(netmask));
memset(dest, 0, sizeof(dest));
addrs = RTA_DST | RTA_NETMASK;
@@ -2412,23 +2434,26 @@ add_default_route(int rdomain, struct in
}
void
-add_static_routes(int rdomain, struct option_data *static_routes)
+add_static_routes(int rdomain, struct in_addr addr, struct in_addr addrmask,
+struct option_data *static_routes)
{
struct in_addr dest, netmask, gateway;
- u_int8_t *addr;
+ u_int8_t *data;
int i;
memset(netmask, 0, sizeof(netmask)); /* Not used for CLASSFULL! */
for (i = 0; (i + 7) static_routes-len; i += 8) {
- addr = static_routes-data[i];
+ data =
Re: dhclient support for /32 assignments
On Tue, Dec 03, 2013 at 04:15:10PM -0800, Matthew Dempsky wrote:
The patch below extends dhclient to mimic this logic from ISC DHCP's
linux script:
if [ x$new_subnet_mask = x255.255.255.255 ] ; then
route add -host $router dev $interface
fi
route add default gw $router $metric_arg dev $interface
With this change, dhclient is able to successfully configure the
network via DHCP on Compute Engine:
$ ifconfig vio0
vio0: flags=8b43UP,BROADCAST,RUNNING,PROMISC,ALLMULTI,SIMPLEX,MULTICAST mtu
1500
lladdr 42:01:0a:f0:8f:56
priority: 0
groups: egress
media: Ethernet autoselect
status: active
inet6 fe80::4001:aff:fef0:8f56%vio0 prefixlen 64 scopeid 0x1
inet 10.240.143.86 netmask 0x
$ netstat -r -n -f inet
Routing tables
Internet:
DestinationGatewayFlags Refs Use Mtu Prio Iface
default10.240.0.1 UGS1 183 - 8 vio0
10.240.0.1 42:01:0a:f0:00:01 UHLc 10 -56 vio0
10.240.0.1/32 link#1 UC 10 -56 vio0
10.240.143.86/32 link#1 UC 00 - 4 vio0
127/8 127.0.0.1 UGRS 00 33144 8 lo0
127.0.0.1 127.0.0.1 UH 10 33144 4 lo0
224/4 127.0.0.1 URS00 33144 8 lo0
ok?
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.268
diff -u -p -r1.268 dhclient.c
--- dhclient.c20 Nov 2013 17:22:46 - 1.268
+++ dhclient.c4 Dec 2013 00:06:08 -
@@ -879,6 +879,21 @@ bind_lease(void)
/* XXX Only use FIRST router address for now. */
memcpy(gateway.s_addr, options[DHO_ROUTERS].data,
options[DHO_ROUTERS].len);
+
+ /*
+ * If we were given a /32 IP assignment, then make sure
+ * the gateway address is routable with equivalent of
+ *
+ * route add -net $gw -netmask 255.255.255.255 \
+ * -cloning -iface $addr
+ */
+ if (mask.s_addr == INADDR_BROADCAST) {
+ add_route(ifi-rdomain, gateway, mask,
+ client-new-address,
+ RTA_DST | RTA_NETMASK | RTA_GATEWAY,
+ RTF_CLONING | RTF_STATIC);
+ }
+
add_default_route(ifi-rdomain, client-new-address,
gateway);
}
Located here, the addition of the 255.255.255.255 route is not done in the
presence of DHO_CLASSLESS_STATIC_ROUTES. As I recall only DHO_ROUTERS and
DHO_STATIC_ROUTES are incompatible with DHO_CLASSLESS_STATIC_ROUTES. So
we may want this chunk to occur unconditionally.
Ken
Re: rename local ticks
On Fri, Nov 29, 2013 at 04:50:17PM -0500, Ted Unangst wrote:
bad form, i think, to have a local variable shadow a global.
I like it. ok krw@
Ken
Index: kern_clock.c
===
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.83
diff -u -p -r1.83 kern_clock.c
--- kern_clock.c 8 Oct 2013 03:50:07 - 1.83
+++ kern_clock.c 29 Nov 2013 21:48:40 -
@@ -213,7 +213,7 @@ int
hzto(const struct timeval *tv)
{
struct timeval now;
- unsigned long ticks;
+ unsigned long nticks;
long sec, usec;
/*
@@ -244,18 +244,18 @@ hzto(const struct timeval *tv)
usec += 100;
}
if (sec 0 || (sec == 0 usec = 0)) {
- ticks = 0;
+ nticks = 0;
} else if (sec = LONG_MAX / 100)
- ticks = (sec * 100 + (unsigned long)usec + (tick - 1))
+ nticks = (sec * 100 + (unsigned long)usec + (tick - 1))
/ tick + 1;
else if (sec = LONG_MAX / hz)
- ticks = sec * hz
+ nticks = sec * hz
+ ((unsigned long)usec + (tick - 1)) / tick + 1;
else
- ticks = LONG_MAX;
- if (ticks INT_MAX)
- ticks = INT_MAX;
- return ((int)ticks);
+ nticks = LONG_MAX;
+ if (nticks INT_MAX)
+ nticks = INT_MAX;
+ return ((int)nticks);
}
/*
@@ -264,7 +264,7 @@ hzto(const struct timeval *tv)
int
tvtohz(const struct timeval *tv)
{
- unsigned long ticks;
+ unsigned long nticks;
time_t sec;
long usec;
@@ -291,18 +291,18 @@ tvtohz(const struct timeval *tv)
sec = tv-tv_sec;
usec = tv-tv_usec;
if (sec 0 || (sec == 0 usec = 0))
- ticks = 0;
+ nticks = 0;
else if (sec = LONG_MAX / 100)
- ticks = (sec * 100 + (unsigned long)usec + (tick - 1))
+ nticks = (sec * 100 + (unsigned long)usec + (tick - 1))
/ tick + 1;
else if (sec = LONG_MAX / hz)
- ticks = sec * hz
+ nticks = sec * hz
+ ((unsigned long)usec + (tick - 1)) / tick + 1;
else
- ticks = LONG_MAX;
- if (ticks INT_MAX)
- ticks = INT_MAX;
- return ((int)ticks);
+ nticks = LONG_MAX;
+ if (nticks INT_MAX)
+ nticks = INT_MAX;
+ return ((int)nticks);
}
int
Re: FDDI/ATM leftovers
On Mon, Nov 18, 2013 at 11:28:56AM +, Alexey E. Suslikov wrote: Martin Pieuchot mpieuchot at nolizard.org writes: - case IFT_FDDI: - case IFT_ATM: case IFT_IEEE1394: any plans for FireWire? :) Nope. :-) Ken
Re: Add fcu(4/macppc) to RAMDISK
On Sat, Nov 09, 2013 at 08:36:23PM +0100, Martin Pieuchot wrote: Without this driver, it's impossible to upgrade my PowerMac7,3 without hearing a fan symphony. ok? As long as all the media still fit this is ok krw@. I don't think there are many in macppc. Ken Index: conf/RAMDISK === RCS file: /cvs/src/sys/arch/macppc/conf/RAMDISK,v retrieving revision 1.97 diff -u -p -r1.97 RAMDISK --- conf/RAMDISK 4 Nov 2013 14:07:16 - 1.97 +++ conf/RAMDISK 9 Nov 2013 18:51:13 - @@ -183,6 +183,26 @@ wi* at uhub?# WaveLAN IEEE 802.11DS #ugen* at uhub?# USB Generic driver umass* at uhub?# USB Mass Storage devices +# I2C bus support +iic* at kiic? +#iic*at piic? +#iic*at smu? + +# I2C devices +#lmtemp* at iic? +#lmenv* at iic? +#maxtmp* at iic? +#adc*at iic? +#tsl*at iic? +#admtmp* at iic? +#pcagpio*at iic? +#gpio* at pcagpio? +#maxds* at iic? +fcu* at iic? +#adt*at iic? +#asms* at iic? +#spdmem* at mem? + # CardBus bus support cardbus* at cardslot? pcmcia* at cardslot?
Re: Fixing an LLVM warning in the i2o code
On Sun, Nov 03, 2013 at 10:51:43PM -0500, Brad Smith wrote:
LLVM errors out on the i2o code with the following warning..
../../../../dev/i2o/iop.c:2399:42: error: comparison of unsigned expression
0 is always false [-Werror,-Wtautological-compare]
pt-pt_nbufs 0 || pt-pt_replylen 0 ||
~~~ ^ ~
Looking at the i2o code it looks as if the pt_replylen field isn't set
anywhere and
doesn't do anything. It looks like it can be garbage collected.
Comments? OK?
When WikiPedia describes i2o as a defunct computer input/output
specification whose SIG was open-source hostile (and was disbanded
in 2000), and which was implemented on only a few server class
machines, it is perhaps past time we lightened the kernel a bit.
:-)
Ken
Index: iop.c
===
RCS file: /home/cvs/src/sys/dev/i2o/iop.c,v
retrieving revision 1.38
diff -u -p -r1.38 iop.c
--- iop.c 30 May 2013 16:15:02 - 1.38
+++ iop.c 4 Nov 2013 03:13:45 -
@@ -2396,8 +2396,9 @@ iop_passthrough(struct iop_softc *sc, st
pt-pt_msglen (letoh16(sc-sc_status.inboundmframesize) 2) ||
pt-pt_msglen sizeof(struct i2o_msg) ||
pt-pt_nbufs IOP_MAX_MSG_XFERS ||
- pt-pt_nbufs 0 || pt-pt_replylen 0 ||
-pt-pt_timo 1000 || pt-pt_timo 5*60*1000)
+ pt-pt_nbufs 0 ||
+ pt-pt_timo 1000 ||
+ pt-pt_timo 5*60*1000)
return (EINVAL);
for (i = 0; i pt-pt_nbufs; i++)
@@ -2446,8 +2447,6 @@ iop_passthrough(struct iop_softc *sc, st
i = (letoh32(im-im_rb-msgflags) 14) ~3;
if (i IOP_MAX_MSG_SIZE)
i = IOP_MAX_MSG_SIZE;
- if (i pt-pt_replylen)
- i = pt-pt_replylen;
if ((rv = copyout(im-im_rb, pt-pt_reply, i)) != 0)
goto bad;
Index: iopio.h
===
RCS file: /home/cvs/src/sys/dev/i2o/iopio.h,v
retrieving revision 1.2
diff -u -p -r1.2 iopio.h
--- iopio.h 26 Jun 2008 05:42:15 - 1.2
+++ iopio.h 4 Nov 2013 03:14:02 -
@@ -57,7 +57,6 @@ struct ioppt {
void*pt_msg;/* pointer to message buffer */
size_t pt_msglen; /* message buffer size in bytes */
void*pt_reply; /* pointer to reply buffer */
- size_t pt_replylen;/* reply buffer size in bytes */
int pt_timo;/* completion timeout in ms */
int pt_nbufs; /* number of transfers */
struct ioppt_buf pt_bufs[IOP_MAX_MSG_XFERS]; /* transfers */
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: Fixing an LLVM warning in the i2o code
On Tue, Nov 05, 2013 at 02:24:22AM +1000, David Gwynne wrote:
On 5 Nov 2013, at 12:40 am, Kenneth R Westerback kwesterb...@rogers.com
wrote:
On Sun, Nov 03, 2013 at 10:51:43PM -0500, Brad Smith wrote:
LLVM errors out on the i2o code with the following warning..
../../../../dev/i2o/iop.c:2399:42: error: comparison of unsigned
expression 0 is always false [-Werror,-Wtautological-compare]
pt-pt_nbufs 0 || pt-pt_replylen 0 ||
~~~ ^ ~
Looking at the i2o code it looks as if the pt_replylen field isn't set
anywhere and
doesn't do anything. It looks like it can be garbage collected.
Comments? OK?
When WikiPedia describes i2o as a defunct computer input/output
specification whose SIG was open-source hostile (and was disbanded
in 2000), and which was implemented on only a few server class
machines, it is perhaps past time we lightened the kernel a bit.
:-)
where there any recent edits to that wikipedia page by a certain mr
westerback?
Were there? I thought I pushed the 'cancel' button. Odd.
Ken
Ken
Index: iop.c
===
RCS file: /home/cvs/src/sys/dev/i2o/iop.c,v
retrieving revision 1.38
diff -u -p -r1.38 iop.c
--- iop.c 30 May 2013 16:15:02 - 1.38
+++ iop.c 4 Nov 2013 03:13:45 -
@@ -2396,8 +2396,9 @@ iop_passthrough(struct iop_softc *sc, st
pt-pt_msglen (letoh16(sc-sc_status.inboundmframesize) 2) ||
pt-pt_msglen sizeof(struct i2o_msg) ||
pt-pt_nbufs IOP_MAX_MSG_XFERS ||
- pt-pt_nbufs 0 || pt-pt_replylen 0 ||
-pt-pt_timo 1000 || pt-pt_timo 5*60*1000)
+ pt-pt_nbufs 0 ||
+ pt-pt_timo 1000 ||
+ pt-pt_timo 5*60*1000)
return (EINVAL);
for (i = 0; i pt-pt_nbufs; i++)
@@ -2446,8 +2447,6 @@ iop_passthrough(struct iop_softc *sc, st
i = (letoh32(im-im_rb-msgflags) 14) ~3;
if (i IOP_MAX_MSG_SIZE)
i = IOP_MAX_MSG_SIZE;
- if (i pt-pt_replylen)
- i = pt-pt_replylen;
if ((rv = copyout(im-im_rb, pt-pt_reply, i)) != 0)
goto bad;
Index: iopio.h
===
RCS file: /home/cvs/src/sys/dev/i2o/iopio.h,v
retrieving revision 1.2
diff -u -p -r1.2 iopio.h
--- iopio.h26 Jun 2008 05:42:15 - 1.2
+++ iopio.h4 Nov 2013 03:14:02 -
@@ -57,7 +57,6 @@ struct ioppt {
void*pt_msg;/* pointer to message buffer */
size_t pt_msglen; /* message buffer size in bytes */
void*pt_reply; /* pointer to reply buffer */
- size_t pt_replylen;/* reply buffer size in bytes */
int pt_timo;/* completion timeout in ms */
int pt_nbufs; /* number of transfers */
struct ioppt_buf pt_bufs[IOP_MAX_MSG_XFERS]; /* transfers */
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: [PATCH] ELF: ensure PT_INTERP strings are NULL-terminated
On Sat, Oct 05, 2013 at 03:22:36PM -0600, Todd C. Miller wrote:
On Wed, 28 Aug 2013 22:34:26 -0400, Kenneth R Westerback wrote:
@@ -552,11 +552,16 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp-p_type == PT_INTERP !interp) {
- if (pp-p_filesz = MAXPATHLEN)
+ if (pp-p_filesz 2 || pp-p_filesz = MAXPATHLEN)
Still think you're depriving yourself of one character out by using
= instead of .
I'm not sure about this. We want to limit the path length to
MAXPATHLEN-1 since we include the NUL terminator in MAXPATHLEN.
The buffer we get from namei_pool is MAXPATHLEN long and the
read_from() function just calls vn_rdwr(). If we allow interp to
be MAXPATHLEN, is there any guarantee that it will end in a NUL
byte?
- todd
My reading at the time convinced me that p_filesz also includes the NUL. So
using = left room for two NULs.
But I am not trying to hold up either version, since I don't really
understand the relevant code. :-)
Ken
Re: re(4) diff that needs testing
On Tue, Oct 01, 2013 at 09:32:30PM +0200, Mark Kettenis wrote: Some re(4) variants now use msi. Unfortunately the interrupt handler isn't careful enough, and we might miss an interrupt. The diff below seems to fix that by disabling the interrupts while processing an interrupt. This is what FreeBSD Linux seem to do. Needs testing on a wide variety of re(4), especially on thoe that don't use msi yet. Index: re.c === RCS file: /cvs/src/sys/dev/ic/re.c,v retrieving revision 1.143 diff -u -p -r1.143 re.c --- re.c 7 Aug 2013 01:06:30 - 1.143 +++ re.c 21 Aug 2013 19:50:39 - @@ -1650,6 +1650,9 @@ re_intr(void *arg) if (!(ifp-if_flags IFF_RUNNING)) return (0); + /* Disable interrupts. */ + CSR_WRITE_2(sc, RL_IMR, 0); + rx = tx = 0; status = CSR_READ_2(sc, RL_ISR); /* If the card has gone away the read returns 0x. */ @@ -1715,6 +1718,8 @@ re_intr(void *arg) if (tx !IFQ_IS_EMPTY(ifp-if_snd)) re_start(ifp); + + CSR_WRITE_2(sc, RL_IMR, sc-rl_intrs); return (claimed); } Also working fine on amd64 with re0 at pci3 dev 0 function 0 Realtek 8101E rev 0x02: RTL8102EL (0x2480), msi, address 3c:4a:92:54:1b:cf rlphy0 at re0 phy 7: RTL8201L 10/100 PHY, rev. 1 In this case the diff seems to have significantly improved performance and latency. Ken
Re: enc interface errno
On Fri, Sep 27, 2013 at 06:56:04PM +0200, Alexander Bluhm wrote: On Fri, Sep 27, 2013 at 12:00:40PM -0400, Kenneth R Westerback wrote: I'm not sure what the 'rule' is regarding ENOMEM and ENOBUFS, but ENOMEN seems more appropriate to me. man 2 errno 12 ENOMEM Cannot allocate memory. The new process image required more memory than was allowed by the hardware or by system-imposed memory management constraints. A lack of swap space is normally temporary; however, a lack of core is not. Soft limits may be increased to their corresponding hard limits. 55 ENOBUFS No buffer space available. An operation on a socket or pipe was not performed because the system lacked sufficient buffer space or because a queue was full. According to this, ENOMEM looks very much like memory allocation failure for the user space. In my case we ran out of network device memory, so I think ENOBUFS is more appropriate. The kernel code is not very consistent there. Developers are tempted to use ENOMEM when malloc(9) fails, but the man page says something different. bluhm bikeshed But how does the failure to allocate a softc relate to a socket or pipe because the system lacked sufficient buffer space? I read 'buffer space' as related to mbufs. Of which I don't think softc's are composed. /bikeshed Ken
Re: Iso image integrity verification
On Thu, Sep 12, 2013 at 10:49:30AM +0200, InterNetX - Robert Garrett wrote: The real problem here is that in order to be added to certain lists of trusted PKI providers, you must be audited by security Assessors one of the things they look for is proof that the software your using isnt tampered with. It appears the OP is trying to solve that issue. EVEN using the CD is not enough to convince some of these people that the software is genuine and untampered with. pgp signed sha256 keys in a public accessible place should do it. Though it would seem to me, that if the sha signature is the same on all the mirrors through openbsds distribution channels that would be verification enough. As then you would have to break into a lot of systems ran by very pedantic, system admins in order to change it on all of them. But let me repeat it isnt the OPS idea of security that is important, its the idea of the people they are paying a lot of money to, and the rules implemented by such companies as Microsoft that are important here. And the ideas of the people they are paying a lot of money to are one or more of a) wrong. b) arbitrary. c) unknown. As you say --- ... should do it.. And how will we know it does it? Who will the security assessors accept as valid guarantors? Theo? Bob? Austin? The Foundation? Resellers? Anybody running a mirror? Some threshold number of developers? There is no entity that owns or can be held responsible for the code, or is capable of providing a solid evidentuary path from commit to your hands. And the OpenBSD community is not some collective Zelig. Ken RG On 09/11/2013 10:10 PM, Valentin Zagura wrote: I was saying that other projects do it in a way they feel comfortable with and maybe you will find a way to do it that you are comfortable with. Using https was one simple idea. I understand that you don't think that this adds any value but maybe there are other ways like signing with PGP, maybe using SSH somehow or having Theo de Raadt saying the SHA checksums on a video on youtube at each release :) or some other simple and effective way that you are comfortable with. I just wanted to point out that one can not easely show his security assessor that it has the right images using some industry standard ways, or someone living in a country that has an oppressive government and would download the image through tor could have some problems if the exit node is malicious. If you feel that any kind of verification is futile, it's ok, that would not stop us from buying the CDs. On Wed, Sep 11, 2013 at 10:32 PM, Kenneth R Westerback kwesterb...@rogers.com wrote: On Wed, Sep 11, 2013 at 08:53:50PM +0300, Valentin Zagura wrote: I don't think I'm more paranoid than the average considering that Debian has a way to do this (http://www.debian.org/CD/verify), fedora has a way to do this (https://fedoraproject.org/verify), even Freebsd has a way to do this ( https://www.freebsd.org/releases/9.1R/announce.html). So you're saying that less paranoid projects are doing it, so why doesn't OpenBSD join the crowd and provide some fuzzy feel good but pointless security theatre? :-) The thought of being more paranoid than an OpenBSD guy is not very comfortable :) Don't worry. You're apparently not paranoid enough yet. The true practical paranoid does not waste time on such mummery. Ken On Wed, Sep 11, 2013 at 8:13 PM, Daniel Bolgheroni dan...@bolgh.eng.br wrote: On Wed, Sep 11, 2013 at 03:17:20PM +0300, Valentin Zagura wrote: Yes, we know, but that file can also be easily compromised if it's not available for download with a secure protocol (HTTPS) If you're paranoid, build your own hardware from the ground up, including designing your own CPU and complementary circuits, download all the sources, audit them all, compile and then run. You can't be fooled by wrong measurements of security.
Re: Iso image integrity verification
On Thu, Sep 12, 2013 at 07:52:22PM +0300, Valentin Zagura wrote: There is no entity that owns or can be held responsible for the code, or is capable of providing a solid evidentuary path from commit to your hands. I thought if we buy the CDs we WILL get a solid evidentuary path from commit to our hands. So this isn't the case? Physical email is as susceptible to MITM attacks as network connections. I know a story of laptops entering the mail system and car springs coming out the other end in the same box. :-) CDs will give you the best evidentuary path available. Compiling everything yourself with a compiler and hardware you built from piles of dirt in a clean room would be better. And then you still have to worry about nano technology being slipped into the dirt. Ken On Wed, Sep 11, 2013 at 1:58 PM, Peter N. M. Hansteen pe...@bsdly.netwrote: On Wed, Sep 11, 2013 at 01:49:14PM +0300, Valentin Zagura wrote: We are going to use a OpenBSD system in a PCI-DSS compliant environment. Is there any way we can prove to our PCI-DSS assessor that the OpenBSD image we use for our installation can be checked so that it is the correct one (is not modified in a malicious way by a third party) ? Probably not what you want to hear, but starting with http://www.openbsd.org/orders.html is usually an excellent idea in this context. Verifiably delivered from a trusted source. A https link to some kind of ISO checksum or something similar (but using strong cryptography) I think would do it, but I could not find any (except a line in the FAQ stating If the men in black suits are out to get you, they're going to get you. which is not the case :) ) It's possible some of the more prominent entries on http://www.openbsd.org/support.html could be persuaded to provide something like that (M:Tier comes to mind, but why are they not on that page?) in exchange for a reasonable fee. But again, for -RELEASE, the CD sets are a good starting point. - Peter -- Peter N. M. Hansteen, member of the first RFC 1149 implementation team http://bsdly.blogspot.com/ http://www.bsdly.net/ http://www.nuug.no/ Remember to set the evil bit on all malicious network traffic delilah spamd[29949]: 85.152.224.147: disconnected after 42673 seconds.
Re: Iso image integrity verification
On Wed, Sep 11, 2013 at 08:53:50PM +0300, Valentin Zagura wrote: I don't think I'm more paranoid than the average considering that Debian has a way to do this (http://www.debian.org/CD/verify), fedora has a way to do this (https://fedoraproject.org/verify), even Freebsd has a way to do this ( https://www.freebsd.org/releases/9.1R/announce.html). So you're saying that less paranoid projects are doing it, so why doesn't OpenBSD join the crowd and provide some fuzzy feel good but pointless security theatre? :-) The thought of being more paranoid than an OpenBSD guy is not very comfortable :) Don't worry. You're apparently not paranoid enough yet. The true practical paranoid does not waste time on such mummery. Ken On Wed, Sep 11, 2013 at 8:13 PM, Daniel Bolgheroni dan...@bolgh.eng.brwrote: On Wed, Sep 11, 2013 at 03:17:20PM +0300, Valentin Zagura wrote: Yes, we know, but that file can also be easily compromised if it's not available for download with a secure protocol (HTTPS) If you're paranoid, build your own hardware from the ground up, including designing your own CPU and complementary circuits, download all the sources, audit them all, compile and then run. You can't be fooled by wrong measurements of security.
Re: Introduce rt_msg() (was nd6_rtmsg)
On Mon, Sep 02, 2013 at 12:43:51PM +0200, Martin Pieuchot wrote:
Diff below is just a small refactoring of two similar code chunks to
inform user processes that something changed regarding a route.
I'd like to get this in because it removes one use of rt_addrinfo in
netinet6.
There's no functional change, ok?
This seems sane. ok krw@ fwiw.
I would suggest copying the 'Inform listeners of the new route'
comment to replace the 'tell the change to user processes watching
the routing socket' comment. The latter reads very oddly to my
native english speaking brain.
Ken
Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.145
diff -u -p -r1.145 route.c
--- net/route.c 28 Aug 2013 06:58:57 - 1.145
+++ net/route.c 2 Sep 2013 10:18:59 -
@@ -346,17 +345,7 @@ rtalloc1(struct sockaddr *dst, int flags
goto miss;
}
/* Inform listeners of the new route */
- bzero(info, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- info.rti_info[RTAX_NETMASK] = rt_mask(rt);
- info.rti_info[RTAX_GATEWAY] = rt-rt_gateway;
- if (rt-rt_ifp != NULL) {
- info.rti_info[RTAX_IFP] =
-
TAILQ_FIRST(rt-rt_ifp-if_addrlist)-ifa_addr;
- info.rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
- }
- rt_missmsg(RTM_ADD, info, rt-rt_flags,
- rt-rt_ifp, 0, tableid);
+ rt_msg(rt, RTM_ADD, tableid);
} else
rt-rt_refcnt++;
} else {
@@ -410,6 +399,25 @@ rtfree(struct rtentry *rt)
Free(rt_key(rt));
pool_put(rtentry_pool, rt);
}
+}
+
+/* tell the change to user processes watching the routing socket. */
+void
+rt_msg(struct rtentry *rt, int cmd, u_int tableid)
+{
+ struct rt_addrinfo info;
+
+ bzero(info, sizeof(info));
+ info.rti_info[RTAX_DST] = rt_key(rt);
+ info.rti_info[RTAX_GATEWAY] = rt-rt_gateway;
+ info.rti_info[RTAX_NETMASK] = rt_mask(rt);
+ if (rt-rt_ifp != NULL) {
+ info.rti_info[RTAX_IFP] =
+ TAILQ_FIRST(rt-rt_ifp-if_addrlist)-ifa_addr;
+ info.rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
+ }
+
+ rt_missmsg(cmd, info, rt-rt_flags, rt-rt_ifp, 0, tableid);
}
void
Index: net/route.h
===
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.78
diff -u -p -r1.78 route.h
--- net/route.h 19 Sep 2012 16:14:01 - 1.78
+++ net/route.h 2 Sep 2013 10:18:59 -
@@ -369,6 +369,7 @@ void rt_ifmsg(struct ifnet *);
void rt_ifannouncemsg(struct ifnet *, int);
void rt_maskedcopy(struct sockaddr *,
struct sockaddr *, struct sockaddr *);
+void rt_msg(struct rtentry *, int, u_int);
void rt_missmsg(int, struct rt_addrinfo *, int, struct ifnet *, int,
u_int);
void rt_newaddrmsg(int, struct ifaddr *, int, struct rtentry *);
Index: netinet6/nd6_rtr.c
===
RCS file: /home/ncvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.72
diff -u -p -r1.72 nd6_rtr.c
--- netinet6/nd6_rtr.c1 Jul 2013 14:22:20 - 1.72
+++ netinet6/nd6_rtr.c2 Sep 2013 10:18:59 -
@@ -70,7 +70,6 @@ void pfxrtr_add(struct nd_prefix *, stru
void pfxrtr_del(struct nd_pfxrouter *);
struct nd_pfxrouter *find_pfxlist_reachable_router(struct nd_prefix *);
void defrouter_delreq(struct nd_defrouter *);
-void nd6_rtmsg(int, struct rtentry *);
void purge_detached(struct ifnet *);
void in6_init_address_ltimes(struct nd_prefix *, struct in6_addrlifetime *);
@@ -410,26 +409,6 @@ nd6_ra_input(struct mbuf *m, int off, in
/*
* default router list processing sub routines
*/
-
-/* tell the change to user processes watching the routing socket. */
-void
-nd6_rtmsg(int cmd, struct rtentry *rt)
-{
- struct rt_addrinfo info;
-
- bzero((caddr_t)info, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- info.rti_info[RTAX_GATEWAY] = rt-rt_gateway;
- info.rti_info[RTAX_NETMASK] = rt_mask(rt);
- if (rt-rt_ifp) {
- info.rti_info[RTAX_IFP] =
- TAILQ_FIRST(rt-rt_ifp-if_addrlist)-ifa_addr;
- info.rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
- }
-
- rt_missmsg(cmd, info, rt-rt_flags, rt-rt_ifp, 0, 0);
-}
-
void
defrouter_addreq(struct nd_defrouter *new)
{
@@ -459,7 +438,7 @@ defrouter_addreq(struct nd_defrouter *ne
error = rtrequest1(RTM_ADD, info, RTP_DEFAULT, newrt,
Re: useradd with empty -k doesn't chown/chmod new home directory
On Sat, Aug 31, 2013 at 06:23:25AM -0600, Todd C. Miller wrote:
Assuming we want to make this a non-fatal error the following should
do.
- todd
Index: usr.sbin/user/user.c
===
RCS file: /home/cvs/openbsd/src/usr.sbin/user/user.c,v
retrieving revision 1.95
diff -u -r1.95 user.c
--- usr.sbin/user/user.c 2 Apr 2013 05:04:47 - 1.95
+++ usr.sbin/user/user.c 31 Aug 2013 12:20:40 -
@@ -288,20 +288,20 @@
{
struct dirent *dp;
DIR *dirp;
- int n;
+ int n = 0;
if ((dirp = opendir(skeldir)) == NULL) {
warn(can't open source . files dir `%s', skeldir);
- return 0;
- }
- for (n = 0; (dp = readdir(dirp)) != NULL n == 0 ; ) {
- if (strcmp(dp-d_name, .) == 0 ||
- strcmp(dp-d_name, ..) == 0) {
- continue;
+ } else {
+ while ((dp = readdir(dirp)) != NULL) {
+ if (strcmp(dp-d_name, .) != 0
+ strcmp(dp-d_name, ..) != 0) {
+ n = 1;
+ break;
+ }
}
- n = 1;
+ (void) closedir(dirp);
}
- (void) closedir(dirp);
if (n == 0) {
warnx(No \dot\ initialisation files found);
} else {
This makes sense to me. ok krw@
Ken
Re: [PATCH] ELF: ensure PT_INTERP strings are NULL-terminated
On Wed, Aug 28, 2013 at 02:54:11PM +0200, Maxime Villard wrote:
Updated diff, with small tweaks from Andres Perera,
* int - size_t, signedness issue, even if it can't be INT_MAX
* NULL - NUL
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.c
--- exec_elf.c4 Jul 2013 17:37:05 - 1.93
+++ exec_elf.c28 Aug 2013 14:36:58 -
@@ -516,7 +516,7 @@ ELFNAME2(exec,makecmds)(struct proc *p,
int error, i;
char *interp = NULL;
u_long pos = 0, phsize;
- size_t randomizequota = ELF_RANDOMIZE_LIMIT;
+ size_t n, randomizequota = ELF_RANDOMIZE_LIMIT;
if (epp-ep_hdrvalid sizeof(Elf_Ehdr))
return (ENOEXEC);
@@ -552,11 +552,21 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp-p_type == PT_INTERP !interp) {
- if (pp-p_filesz = MAXPATHLEN)
+ if (pp-p_filesz = MAXPATHLEN ||
+ pp-p_filesz 2)
Since p_filesz includes the NUL and MAXPATHLEN includes the NUL as
far as I know, could the comparison not be simply ' MAXPATHLEN'?
In passing I note that 37 out of 749 declarations using MAXPATHLEN in the
tree use 'char blah[MAXPATHLEN + 1]'. No idea if it is worth looking at
making them do without the '+ 1'. :-)
I also note in passing several #define PATH_MAX (MAXPATHLEN + 1)'
declarations, which strike me as odd since MAXPATHLEN is defined
in sys/param.h by '#define MAXPATHEN PATH_MAX'.
Let the POSIX lawyers apply any necessary cluebats please.
goto bad;
interp = pool_get(namei_pool, PR_WAITOK);
if ((error = ELFNAME(read_from)(p, epp-ep_vp,
pp-p_offset, interp, pp-p_filesz)) != 0) {
+ goto bad;
+ }
+ /* Ensure interp is a valid, NUL-terminated string */
The condition is not that the string is NUL terminated (aaa\0aaa\0 is after
all NUL terminated and valid as far as any function reading strings would be
concerned. It just has trailing garbage.) Perhaps the comment should refer
to making sure the string is spec compliant by being of the expected length.
Ken
+ for (n = 0; n pp-p_filesz; n++) {
+ if (interp[n] == '\0')
+ break;
+ }
+ if (n != pp-p_filesz - 1) {
+ error = ENOEXEC;
goto bad;
}
} else if (pp-p_type == PT_LOAD) {
On 08/28/13 08:44, Maxime Villard wrote:
Hi,
in the ELF format, the PT_INTERP segment contains the path of the
interpreter which must be loaded. For this segment, the kernel looks
at these values in the program header:
p_offset: offset of the path string
p_filesz: size of the path string, including the \0
The path string must be a valid string, or the binary won't be loaded.
The kernel actually doesn't check the string, it just reads p_filesz
bytes from p_offset, and later it checks if it can be loaded. Malformed
binaries which have paths like:
a
or
aaa\0aaa\0aaa\0aaa\0
are parsed, loaded, and then the kernel figures out that the path is
not valid, and aborts.
When the kernel reads the path from the file, it should check directly
the string to ensure it is at least a valid string.
Here is a patch for this. For pp-p_filesz, I could have put
if (pp-p_filesz == 0)
but values of 1 also don't make sense; \0 can't be a valid path.
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.c
--- exec_elf.c 4 Jul 2013 17:37:05 - 1.93
+++ exec_elf.c 27 Aug 2013 22:59:21 -
@@ -513,7 +513,7 @@ ELFNAME2(exec,makecmds)(struct proc *p,
Elf_Ehdr *eh = epp-ep_hdr;
Elf_Phdr *ph, *pp, *base_ph = NULL;
Elf_Addr phdr = 0, exe_base = 0;
- int error, i;
+ int error, i, n;
char *interp = NULL;
u_long pos = 0, phsize;
size_t randomizequota = ELF_RANDOMIZE_LIMIT;
@@ -552,11 +552,21 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp-p_type == PT_INTERP !interp) {
- if (pp-p_filesz = MAXPATHLEN)
+ if (pp-p_filesz = MAXPATHLEN ||
+ pp-p_filesz 2)
goto bad;
interp = pool_get(namei_pool, PR_WAITOK);
if ((error = ELFNAME(read_from)(p, epp-ep_vp,
Re: [PATCH] ELF: ensure PT_INTERP strings are NULL-terminated
On Wed, Aug 28, 2013 at 08:44:26PM +0200, Maxime Villard wrote:
On 08/28/13 16:30, Kenneth R Westerback wrote:
On Wed, Aug 28, 2013 at 02:54:11PM +0200, Maxime Villard wrote:
Updated diff, with small tweaks from Andres Perera,
* int - size_t, signedness issue, even if it can't be INT_MAX
* NULL - NUL
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.c
--- exec_elf.c 4 Jul 2013 17:37:05 - 1.93
+++ exec_elf.c 28 Aug 2013 14:36:58 -
@@ -516,7 +516,7 @@ ELFNAME2(exec,makecmds)(struct proc *p,
int error, i;
char *interp = NULL;
u_long pos = 0, phsize;
- size_t randomizequota = ELF_RANDOMIZE_LIMIT;
+ size_t n, randomizequota = ELF_RANDOMIZE_LIMIT;
if (epp-ep_hdrvalid sizeof(Elf_Ehdr))
return (ENOEXEC);
@@ -552,11 +552,21 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp-p_type == PT_INTERP !interp) {
- if (pp-p_filesz = MAXPATHLEN)
+ if (pp-p_filesz = MAXPATHLEN ||
+ pp-p_filesz 2)
Since p_filesz includes the NUL and MAXPATHLEN includes the NUL as
far as I know, could the comparison not be simply ' MAXPATHLEN'?
In passing I note that 37 out of 749 declarations using MAXPATHLEN in the
tree use 'char blah[MAXPATHLEN + 1]'. No idea if it is worth looking at
making them do without the '+ 1'. :-)
I also note in passing several #define PATH_MAX (MAXPATHLEN + 1)'
declarations, which strike me as odd since MAXPATHLEN is defined
in sys/param.h by '#define MAXPATHEN PATH_MAX'.
Let the POSIX lawyers apply any necessary cluebats please.
goto bad;
interp = pool_get(namei_pool, PR_WAITOK);
if ((error = ELFNAME(read_from)(p, epp-ep_vp,
pp-p_offset, interp, pp-p_filesz)) != 0) {
+ goto bad;
+ }
+ /* Ensure interp is a valid, NUL-terminated string */
The condition is not that the string is NUL terminated (aaa\0aaa\0 is
after
all NUL terminated and valid as far as any function reading strings would be
concerned. It just has trailing garbage.) Perhaps the comment should refer
to making sure the string is spec compliant by being of the expected length.
By 'valid string' I actually meant 'without trailing garbage'... but ok
if it's not clear
/* Ensure interp is NUL-terminated and of the expected length */
That would be clear to me.
Ken
Ken
+ for (n = 0; n pp-p_filesz; n++) {
+ if (interp[n] == '\0')
+ break;
+ }
+ if (n != pp-p_filesz - 1) {
+ error = ENOEXEC;
goto bad;
}
} else if (pp-p_type == PT_LOAD) {
On 08/28/13 08:44, Maxime Villard wrote:
Hi,
in the ELF format, the PT_INTERP segment contains the path of the
interpreter which must be loaded. For this segment, the kernel looks
at these values in the program header:
p_offset: offset of the path string
p_filesz: size of the path string, including the \0
The path string must be a valid string, or the binary won't be loaded.
The kernel actually doesn't check the string, it just reads p_filesz
bytes from p_offset, and later it checks if it can be loaded. Malformed
binaries which have paths like:
a
or
aaa\0aaa\0aaa\0aaa\0
are parsed, loaded, and then the kernel figures out that the path is
not valid, and aborts.
When the kernel reads the path from the file, it should check directly
the string to ensure it is at least a valid string.
Here is a patch for this. For pp-p_filesz, I could have put
if (pp-p_filesz == 0)
but values of 1 also don't make sense; \0 can't be a valid path.
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.c
--- exec_elf.c4 Jul 2013 17:37:05 - 1.93
+++ exec_elf.c27 Aug 2013 22:59:21 -
@@ -513,7 +513,7 @@ ELFNAME2(exec,makecmds)(struct proc *p,
Elf_Ehdr *eh = epp-ep_hdr;
Elf_Phdr *ph, *pp, *base_ph = NULL;
Elf_Addr phdr = 0, exe_base = 0;
- int error, i;
+ int error, i, n;
char *interp = NULL;
u_long pos = 0, phsize;
size_t randomizequota = ELF_RANDOMIZE_LIMIT;
@@ -552,11 +552,21 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp
Re: [PATCH] ELF: ensure PT_INTERP strings are NULL-terminated
On Wed, Aug 28, 2013 at 09:43:24PM +0200, Maxime Villard wrote:
On 08/28/13 20:57, Matthew Dempsky wrote:
On Wed, Aug 28, 2013 at 5:54 AM, Maxime Villard m...@m00nbsd.net wrote:
+ /* Ensure interp is a valid, NUL-terminated string
*/
+ for (n = 0; n pp-p_filesz; n++) {
+ if (interp[n] == '\0')
+ break;
+ }
+ if (n != pp-p_filesz - 1) {
+ error = ENOEXEC;
goto bad;
}
A more concise test would be:
if (strnlen(interp, pp-p_filesz) != pp-p_filesz - 1) {
error = ENOEXEC;
goto bad;
}
Hum you're right, it's better that way
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -p -r1.93 exec_elf.c
--- exec_elf.c4 Jul 2013 17:37:05 - 1.93
+++ exec_elf.c28 Aug 2013 21:14:22 -
@@ -552,11 +552,16 @@ ELFNAME2(exec,makecmds)(struct proc *p,
for (i = 0, pp = ph; i eh-e_phnum; i++, pp++) {
if (pp-p_type == PT_INTERP !interp) {
- if (pp-p_filesz = MAXPATHLEN)
+ if (pp-p_filesz 2 || pp-p_filesz = MAXPATHLEN)
Still think you're depriving yourself of one character out by using
= instead of .
Ken
goto bad;
interp = pool_get(namei_pool, PR_WAITOK);
if ((error = ELFNAME(read_from)(p, epp-ep_vp,
pp-p_offset, interp, pp-p_filesz)) != 0) {
+ goto bad;
+ }
+ /* Ensure interp is NUL-terminated and of the expected
length */
+ if (strnlen(interp, pp-p_filesz) != pp-p_filesz - 1) {
+ error = ENOEXEC;
goto bad;
}
} else if (pp-p_type == PT_LOAD) {
It no longer looks like my patch...
Re: Remove unused argument from *rtrequest()
On Tue, Aug 27, 2013 at 03:58:51PM +0200, Martin Pieuchot wrote:
In order to define a proper API for our routine table, I'd like to turn
the struct rt_addrinfo into a private type (ie: only used in route.c
and rtsock.c).
This type is used by a lost of code in our network stack to add or delete
a route but also in the various *rtrequest() functions. However in
these functions the argument is never used! So the diff below kills it.
ok?
Less is more. ok krw@.
Ken
Index: net/if.c
===
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c 20 Aug 2013 09:14:22 - 1.262
+++ net/if.c 27 Aug 2013 13:50:50 -
@@ -985,7 +985,7 @@ ifaof_ifpforaddr(struct sockaddr *addr,
* This should be moved to /sys/net/link.c eventually.
*/
void
-link_rtrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+link_rtrequest(int cmd, struct rtentry *rt)
{
struct ifaddr *ifa;
struct sockaddr *dst;
@@ -999,7 +999,7 @@ link_rtrequest(int cmd, struct rtentry *
ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
if (ifa-ifa_rtrequest ifa-ifa_rtrequest != link_rtrequest)
- ifa-ifa_rtrequest(cmd, rt, info);
+ ifa-ifa_rtrequest(cmd, rt);
}
}
Index: net/if.h
===
RCS file: /home/ncvs/src/sys/net/if.h,v
retrieving revision 1.144
diff -u -p -r1.144 if.h
--- net/if.h 20 Jun 2013 12:03:40 - 1.144
+++ net/if.h 27 Aug 2013 13:50:50 -
@@ -494,7 +494,7 @@ struct ifaddr {
struct ifnet *ifa_ifp; /* back-pointer to interface */
TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
/* check or clean routes (+ or -)'d */
- void(*ifa_rtrequest)(int, struct rtentry *, struct rt_addrinfo *);
+ void(*ifa_rtrequest)(int, struct rtentry *);
u_int ifa_flags; /* mostly rt_flags for cloning */
u_int ifa_refcnt; /* count of references */
int ifa_metric; /* cost of going out this interface */
@@ -842,7 +842,7 @@ structifaddr *ifa_ifwithroute(int, stru
struct sockaddr *, u_int);
struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
void ifafree(struct ifaddr *);
-void link_rtrequest(int, struct rtentry *, struct rt_addrinfo *);
+void link_rtrequest(int, struct rtentry *);
void if_clone_attach(struct if_clone *);
void if_clone_detach(struct if_clone *);
@@ -858,7 +858,7 @@ int loioctl(struct ifnet *, u_long, cadd
void loopattach(int);
int looutput(struct ifnet *,
struct mbuf *, struct sockaddr *, struct rtentry *);
-void lortrequest(int, struct rtentry *, struct rt_addrinfo *);
+void lortrequest(int, struct rtentry *);
void ifa_add(struct ifnet *, struct ifaddr *);
void ifa_del(struct ifnet *, struct ifaddr *);
void ifa_update_broadaddr(struct ifnet *, struct ifaddr *,
Index: net/if_loop.c
===
RCS file: /home/ncvs/src/sys/net/if_loop.c,v
retrieving revision 1.49
diff -u -p -r1.49 if_loop.c
--- net/if_loop.c 28 Mar 2013 16:55:27 - 1.49
+++ net/if_loop.c 27 Aug 2013 13:50:50 -
@@ -373,7 +373,7 @@ lo_altqstart(struct ifnet *ifp)
/* ARGSUSED */
void
-lortrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+lortrequest(int cmd, struct rtentry *rt)
{
if (rt)
rt-rt_rmx.rmx_mtu = LOMTU;
Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:50:50 -
@@ -781,7 +781,7 @@ rtrequest1(int req, struct rt_addrinfo *
rt-rt_flags = ~RTF_UP;
if ((ifa = rt-rt_ifa) ifa-ifa_rtrequest)
- ifa-ifa_rtrequest(RTM_DELETE, rt, info);
+ ifa-ifa_rtrequest(RTM_DELETE, rt);
rttrash++;
if (ret_nrt)
@@ -925,14 +925,13 @@ rtrequest1(int req, struct rt_addrinfo *
was (%p)\n, ifa, (*ret_nrt)-rt_ifa);
if ((*ret_nrt)-rt_ifa-ifa_rtrequest)
(*ret_nrt)-rt_ifa-ifa_rtrequest(
- RTM_DELETE, *ret_nrt, NULL);
+ RTM_DELETE, *ret_nrt);
ifafree((*ret_nrt)-rt_ifa);
(*ret_nrt)-rt_ifa = ifa;
(*ret_nrt)-rt_ifp = ifa-ifa_ifp;
Re: defer routing table updates on link state changes
On Tue, Aug 27, 2013 at 01:54:34PM +0200, Mike Belopuhov wrote: On 27 August 2013 13:39, Martin Pieuchot mpieuc...@nolizard.org wrote: I think that's the right approach but the current code generating interfaces indexes is too clever from my point of view, it tries to reuse the last index if possible. This could lead to some funny races if we detach and reattach an interface before the task get scheduled. So I'm ok with your diff if we avoid to reuse the last index too soon. Diff below does that. i'm ok with this change. I like that tweak too. Makes me feel warm and safe. Ken
Re: Split rtinit()
On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote:
So I started to play with the routine table and I'm slowly trying to
unify the various code paths to add and delete route entries. The
diff below is a first step, it splits rtinit() into rt_add() and
rt_delete() there should be no functional change.
ok?
That makes s much more sense. :-).
ok krw@ (note: not a routing table guru!)
Ken
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c 20 Aug 2013 09:14:22 - 1.262
+++ net/if.c 27 Aug 2013 13:33:03 -
@@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
return;
s = splnet();
- rtinit(ifa, RTM_DELETE, 0);
+ rt_del(ifa, 0);
#if 0
ifa_del(ifp, ifa);
ifp-if_lladdr = NULL;
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:33:03 -
@@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
* for an interface.
*/
int
-rtinit(struct ifaddr *ifa, int cmd, int flags)
+rt_add(struct ifaddr *ifa, int flags)
{
struct rtentry *rt;
- struct sockaddr *dst, *deldst;
- struct mbuf *m = NULL;
+ struct sockaddr *dst;
struct rtentry *nrt = NULL;
int error;
struct rt_addrinfo info;
@@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
u_short rtableid = ifa-ifa_ifp-if_rdomain;
dst = flags RTF_HOST ? ifa-ifa_dstaddr : ifa-ifa_addr;
- if (cmd == RTM_DELETE) {
- if ((flags RTF_HOST) == 0 ifa-ifa_netmask) {
- m = m_get(M_DONTWAIT, MT_SONAME);
- if (m == NULL)
- return (ENOBUFS);
- deldst = mtod(m, struct sockaddr *);
- rt_maskedcopy(dst, deldst, ifa-ifa_netmask);
- dst = deldst;
- }
- if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
- rt-rt_refcnt--;
- /* try to find the right route */
- while (rt rt-rt_ifa != ifa)
- rt = (struct rtentry *)
- ((struct radix_node *)rt)-rn_dupedkey;
- if (!rt) {
- if (m != NULL)
- (void) m_free(m);
- return (flags RTF_HOST ? EHOSTUNREACH
- : ENETUNREACH);
- }
- }
- }
bzero(info, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | ifa-ifa_flags;
info.rti_info[RTAX_DST] = dst;
- if (cmd == RTM_ADD)
- info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
+ info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
info.rti_info[RTAX_LABEL] =
rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
@@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
* change it to meet bsdi4 behavior.
*/
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
- error = rtrequest1(cmd, info, RTP_CONNECTED, nrt, rtableid);
- if (cmd == RTM_DELETE) {
- if (error == 0 (rt = nrt) != NULL) {
- rt_newaddrmsg(cmd, ifa, error, nrt);
- if (rt-rt_refcnt = 0) {
- rt-rt_refcnt++;
- rtfree(rt);
- }
- }
- if (m != NULL)
- (void) m_free(m);
- }
- if (cmd == RTM_ADD error == 0 (rt = nrt) != NULL) {
+ error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, nrt, rtableid);
+ if (error == 0 (rt = nrt) != NULL) {
rt-rt_refcnt--;
if (rt-rt_ifa != ifa) {
- printf(rtinit: wrong ifa (%p) was (%p)\n,
+ printf(%s: wrong ifa (%p) was (%p)\n, __func__,
ifa, rt-rt_ifa);
if (rt-rt_ifa-ifa_rtrequest)
rt-rt_ifa-ifa_rtrequest(RTM_DELETE, rt, NULL);
@@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
if (ifa-ifa_rtrequest)
ifa-ifa_rtrequest(RTM_ADD, rt, NULL);
}
- rt_newaddrmsg(cmd, ifa, error, nrt);
+ rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ }
+
+ return (error);
+}
+
+int
+rt_del(struct ifaddr *ifa, int flags)
+{
+ struct rtentry *rt;
+
Re: ldconfig/prebind.c - remove dead assignments
On Sun, Jul 14, 2013 at 09:23:53AM +0200, J??r??mie Courr??ges-Anglas wrote:
David Hill dh...@mindcry.org writes:
remove unused variables.
Makes sense. ok?
Index: ldconfig/prebind.c
===
RCS file: /cvs/src/libexec/ld.so/ldconfig/prebind.c,v
retrieving revision 1.20
diff -u -p -r1.20 prebind.c
--- ldconfig/prebind.c 4 May 2013 09:23:33 - 1.20
+++ ldconfig/prebind.c 5 Jul 2013 18:18:24 -
@@ -472,12 +472,10 @@ done:
int
elf_check_note(void *buf, Elf_Phdr *phdr)
{
- Elf_Ehdr *ehdr;
u_long address;
u_int *pint;
char *osname;
- ehdr = (Elf_Ehdr *)buf;
address = phdr-p_offset;
pint = (u_int *)((char *)buf + address);
osname = (char *)buf + address + sizeof(*pint) * 3;
@@ -1712,7 +1710,6 @@ elf_write_lib(struct elf_object *object,
u_int32_t next_start, *fixuptab = NULL;
struct stat ifstat;
off_t base_offset;
- size_t len;
int fd = -1, i;
int readonly = 0;
@@ -1729,7 +1726,7 @@ elf_write_lib(struct elf_object *object,
readonly = 1;
}
lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
- len = read(fd, footer, sizeof(struct prebind_footer));
+ read(fd, footer, sizeof(struct prebind_footer));
Here I would consider actually using len to check for failure. And of
course changing the type of len to ssize_t to allow such checking.
Ken
if (fstat(fd, ifstat) == -1) {
perror(object-load_name);
@@ -2210,7 +2207,6 @@ void
copy_oldsymcache(int objidx, void *prebind_map)
{
struct prebind_footer *footer;
- struct elf_object *object;
struct elf_object *tobj;
struct symcache_noflag *tcache;
struct symcachetab *symcache;
@@ -2219,8 +2215,6 @@ copy_oldsymcache(int objidx, void *prebi
u_int32_t offset;
u_int32_t *poffset;
struct nameidx *nameidx;
-
- object = objarray[objidx].obj;
poffset = (u_int32_t *)prebind_map;
c = prebind_map;
Re: ldd.c - plug memleak
On Sun, Jul 14, 2013 at 09:23:05AM +0200, J??r??mie Courr??ges-Anglas wrote:
David Hill dh...@mindcry.org writes:
Hello -
Hi,
doit() was not free()'ing memory or close()'ing the file descriptor if
realpath() failed or dlopen() returned NULL.
This diff just moves close() and free() up once we are done using them.
Makes sense. ok?
Makes sense to me too. ok krw@
Ken
Index: ldd/ldd.c
===
RCS file: /cvs/src/libexec/ld.so/ldd/ldd.c,v
retrieving revision 1.15
diff -u -p -r1.15 ldd.c
--- ldd/ldd.c 29 Apr 2011 07:19:19 - 1.15
+++ ldd/ldd.c 5 Jul 2013 18:12:43 -
@@ -126,12 +126,14 @@ doit(char *name)
free(phdr);
return 1;
}
+ close(fd);
for (i = 0; i ehdr.e_phnum; i++)
if (phdr[i].p_type == PT_INTERP) {
interp = 1;
break;
}
+ free(phdr);
if (ehdr.e_type == ET_DYN !interp) {
printf(%s:\n, name);
@@ -144,13 +146,8 @@ doit(char *name)
printf(%s\n, dlerror());
return 1;
}
- close(fd);
- free(phdr);
return 0;
}
-
- close(fd);
- free(phdr);
if (i == ehdr.e_phnum) {
warnx(%s: not a dynamic executable, name);
Re: ldconfig/prebind.c - remove dead assignments
On Sun, Jul 14, 2013 at 03:13:32PM +0200, J??r??mie Courr??ges-Anglas wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 09:23:53AM +0200, J??r??mie Courr??ges-Anglas wrote:
David Hill dh...@mindcry.org writes:
remove unused variables.
Makes sense. ok?
[...]
lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
-len = read(fd, footer, sizeof(struct prebind_footer));
+read(fd, footer, sizeof(struct prebind_footer));
Here I would consider actually using len to check for failure. And of
course changing the type of len to ssize_t to allow such checking.
Ken
Sure (assuming that an undetected lseek() error would be caught by
read()).
I guess lseek() should also have a result check, as the rabbit hole
yawns wider. :-) But to concentrate on the read, I think the other
error to check for is a short read. But not being a ld.so hacker I
have no feel for how much trouble could be caused by only reading
in a partial object. I'm guessing the file would have to be truly
pathological.
Ken
Index: prebind.c
===
RCS file: /cvs/src/libexec/ld.so/ldconfig/prebind.c,v
retrieving revision 1.21
diff -u -p -r1.21 prebind.c
--- prebind.c 5 Jul 2013 21:10:50 - 1.21
+++ prebind.c 14 Jul 2013 13:04:19 -
@@ -475,12 +475,10 @@ done:
int
elf_check_note(void *buf, Elf_Phdr *phdr)
{
- Elf_Ehdr *ehdr;
u_long address;
u_int *pint;
char *osname;
- ehdr = (Elf_Ehdr *)buf;
address = phdr-p_offset;
pint = (u_int *)((char *)buf + address);
osname = (char *)buf + address + sizeof(*pint) * 3;
@@ -1715,7 +1713,7 @@ elf_write_lib(struct elf_object *object,
u_int32_t next_start, *fixuptab = NULL;
struct stat ifstat;
off_t base_offset;
- size_t len;
+ ssize_t len;
int fd = -1, i;
int readonly = 0;
@@ -1733,6 +1731,11 @@ elf_write_lib(struct elf_object *object,
}
lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
len = read(fd, footer, sizeof(struct prebind_footer));
+ if (len == -1) {
+ perror(object-load_name);
+ close(fd);
+ return 1;
+ }
if (fstat(fd, ifstat) == -1) {
perror(object-load_name);
@@ -2213,7 +2216,6 @@ void
copy_oldsymcache(int objidx, void *prebind_map)
{
struct prebind_footer *footer;
- struct elf_object *object;
struct elf_object *tobj;
struct symcache_noflag *tcache;
struct symcachetab *symcache;
@@ -,8 +2224,6 @@ copy_oldsymcache(int objidx, void *prebi
u_int32_t offset;
u_int32_t *poffset;
struct nameidx *nameidx;
-
- object = objarray[objidx].obj;
poffset = (u_int32_t *)prebind_map;
c = prebind_map;
Re: ldconfig/prebind.c - remove dead assignments
On Sun, Jul 14, 2013 at 05:56:46PM +0200, J??r??mie Courr??ges-Anglas wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 03:13:32PM +0200, J??r??mie Courr??ges-Anglas wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 09:23:53AM +0200, J??r??mie Courr??ges-Anglas
wrote:
David Hill dh...@mindcry.org writes:
remove unused variables.
Makes sense. ok?
[...]
lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
- len = read(fd, footer, sizeof(struct prebind_footer));
+ read(fd, footer, sizeof(struct prebind_footer));
Here I would consider actually using len to check for failure. And of
course changing the type of len to ssize_t to allow such checking.
Ken
Sure (assuming that an undetected lseek() error would be caught by
read()).
I guess lseek() should also have a result check, as the rabbit hole
yawns wider. :-)
So let's do that.
But to concentrate on the read, I think the other
error to check for is a short read. But not being a ld.so hacker I
have no feel for how much trouble could be caused by only reading
in a partial object. I'm guessing the file would have to be truly
pathological.
I wondered about that case too, but got lazy. Turns out footer is 80
bytes, while black box testing shows that this code path isn't hit
for files invalid - not looking like ELF enough or smaller than 475
bytes (on my i386 machine).
In any case, I don't think a check hurts much.
(elf_sum_reloc() ansified while here, must stop adding stuff over diffs...)
Index: prebind.c
===
RCS file: /cvs/src/libexec/ld.so/ldconfig/prebind.c,v
retrieving revision 1.21
diff -u -p -p -u -r1.21 prebind.c
--- prebind.c 5 Jul 2013 21:10:50 - 1.21
+++ prebind.c 14 Jul 2013 15:50:24 -
@@ -152,7 +152,7 @@ struct elf_object * elf_lookup_object_de
void elf_free_curbin_list(struct elf_object *obj);
void elf_resolve_curbin(void);
struct proglist *elf_newbin(void);
-void elf_sum_reloc();
+void elf_sum_reloc(void);
int elf_prep_lib_prebind(struct elf_object *object);
int elf_prep_bin_prebind(struct proglist *pl);
void add_fixup_prog(struct elf_object *prog, struct elf_object *obj, int idx,
@@ -475,12 +475,10 @@ done:
int
elf_check_note(void *buf, Elf_Phdr *phdr)
{
- Elf_Ehdr *ehdr;
u_long address;
u_int *pint;
char *osname;
- ehdr = (Elf_Ehdr *)buf;
address = phdr-p_offset;
pint = (u_int *)((char *)buf + address);
osname = (char *)buf + address + sizeof(*pint) * 3;
@@ -1425,7 +1423,7 @@ elf_init_objarray(void)
}
void
-elf_sum_reloc()
+elf_sum_reloc(void)
{
int numobjs;
int err = 0;
@@ -1715,7 +1713,7 @@ elf_write_lib(struct elf_object *object,
u_int32_t next_start, *fixuptab = NULL;
struct stat ifstat;
off_t base_offset;
- size_t len;
+ ssize_t len;
int fd = -1, i;
int readonly = 0;
@@ -1731,8 +1729,24 @@ elf_write_lib(struct elf_object *object,
}
readonly = 1;
}
- lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
+ if (lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END)
+ == -1) {
+ perror(object-load_name);
+ close(fd);
+ return 1;
+ }
+
len = read(fd, footer, sizeof(struct prebind_footer));
+ if (len = -1 len sizeof(struct prebind_footer)) {
I think this condition is incorrect.
if (len == -1 || len sizeof())
perhaps?
Ken
+ close(fd);
+ if (len == -1)
+ perror(object-load_name);
+ else
+ /* paranoia */
+ warnx(%s on %s: short read (corrupted file?),
+ __func__, object-load_name);
+ return 1;
+ }
if (fstat(fd, ifstat) == -1) {
perror(object-load_name);
@@ -2213,7 +2227,6 @@ void
copy_oldsymcache(int objidx, void *prebind_map)
{
struct prebind_footer *footer;
- struct elf_object *object;
struct elf_object *tobj;
struct symcache_noflag *tcache;
struct symcachetab *symcache;
@@ -,8 +2235,6 @@ copy_oldsymcache(int objidx, void *prebi
u_int32_t offset;
u_int32_t *poffset;
struct nameidx *nameidx;
-
- object = objarray[objidx].obj;
poffset = (u_int32_t *)prebind_map;
c = prebind_map;
Re: ldconfig/prebind.c - remove dead assignments
On Sun, Jul 14, 2013 at 08:22:45PM +0200, J??r??mie Courr??ges-Anglas wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 05:56:46PM +0200, J??r??mie Courr??ges-Anglas wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 03:13:32PM +0200, J??r??mie Courr??ges-Anglas
wrote:
Kenneth R Westerback kwesterb...@rogers.com writes:
On Sun, Jul 14, 2013 at 09:23:53AM +0200, J??r??mie Courr??ges-Anglas
wrote:
David Hill dh...@mindcry.org writes:
remove unused variables.
Makes sense. ok?
[...]
lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
- len = read(fd, footer, sizeof(struct prebind_footer));
+ read(fd, footer, sizeof(struct prebind_footer));
Here I would consider actually using len to check for failure. And of
course changing the type of len to ssize_t to allow such checking.
Ken
Sure (assuming that an undetected lseek() error would be caught by
read()).
I guess lseek() should also have a result check, as the rabbit hole
yawns wider. :-)
So let's do that.
But to concentrate on the read, I think the other
error to check for is a short read. But not being a ld.so hacker I
have no feel for how much trouble could be caused by only reading
in a partial object. I'm guessing the file would have to be truly
pathological.
I wondered about that case too, but got lazy. Turns out footer is 80
bytes, while black box testing shows that this code path isn't hit
for files invalid - not looking like ELF enough or smaller than 475
bytes (on my i386 machine).
Hmm, I should have added that feeding it a valid executable truncated to
475 bytes or more produces reproducible segfaults. O:)
In any case, I don't think a check hurts much.
(elf_sum_reloc() ansified while here, must stop adding stuff over diffs...)
Index: prebind.c
===
RCS file: /cvs/src/libexec/ld.so/ldconfig/prebind.c,v
retrieving revision 1.21
diff -u -p -p -u -r1.21 prebind.c
--- prebind.c 5 Jul 2013 21:10:50 - 1.21
+++ prebind.c 14 Jul 2013 15:50:24 -
@@ -152,7 +152,7 @@ struct elf_object * elf_lookup_object_de
void elf_free_curbin_list(struct elf_object *obj);
void elf_resolve_curbin(void);
struct proglist *elf_newbin(void);
-void elf_sum_reloc();
+void elf_sum_reloc(void);
int elf_prep_lib_prebind(struct elf_object *object);
int elf_prep_bin_prebind(struct proglist *pl);
void add_fixup_prog(struct elf_object *prog, struct elf_object *obj,
int idx,
@@ -475,12 +475,10 @@ done:
int
elf_check_note(void *buf, Elf_Phdr *phdr)
{
- Elf_Ehdr *ehdr;
u_long address;
u_int *pint;
char *osname;
- ehdr = (Elf_Ehdr *)buf;
address = phdr-p_offset;
pint = (u_int *)((char *)buf + address);
osname = (char *)buf + address + sizeof(*pint) * 3;
@@ -1425,7 +1423,7 @@ elf_init_objarray(void)
}
void
-elf_sum_reloc()
+elf_sum_reloc(void)
{
int numobjs;
int err = 0;
@@ -1715,7 +1713,7 @@ elf_write_lib(struct elf_object *object,
u_int32_t next_start, *fixuptab = NULL;
struct stat ifstat;
off_t base_offset;
- size_t len;
+ ssize_t len;
int fd = -1, i;
int readonly = 0;
@@ -1731,8 +1729,24 @@ elf_write_lib(struct elf_object *object,
}
readonly = 1;
}
- lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END);
+ if (lseek(fd, -((off_t)sizeof(struct prebind_footer)), SEEK_END)
+ == -1) {
+ perror(object-load_name);
+ close(fd);
+ return 1;
+ }
+
len = read(fd, footer, sizeof(struct prebind_footer));
+ if (len = -1 len sizeof(struct prebind_footer)) {
I think this condition is incorrect.
It checks whether len is in the [-1; 80] range. I was perhaps a bit too
paranoid about the fact that read(2) can return a value between
SSIZE_MAX and SIZE_MAX -2 (see CAVEATS). Since I've read this section
I kinda wired it into my brain; I don't even know if that applies to
OpenBSD.
if (len == -1 || len sizeof())
Of course here the maximum (and desired) return value is 80 so we won't
hit that problem. Maybe a clearer check would be:
if (len != sizeof()) {
if (len == -1)
/* handle error */
else
/* handle short read */
}
which I find clearer.
Does this look reasonable?
Works for me. ok krw@
Ken
Index: prebind.c
===
RCS file: /cvs/src/libexec/ld.so/ldconfig/prebind.c,v
retrieving revision 1.21
diff -u -p -p -u -r1.21 prebind.c
--- prebind.c 5 Jul 2013 21:10:50 - 1.21
+++ prebind.c 14 Jul 2013 18:18:53 -
@@ -152,7 +152,7 @@ struct
Re: exec_elf.c: mistake ?
On Sat, Jul 06, 2013 at 05:21:31PM +0200, Maxime Villard wrote: Hi, - - - - sys/kern/exec_elf.c l.236 ~ 251252 Are my code scanner and me wrong, or 'bdiff' may not be initialized ? Codewise it does look possible that bdiff will be used uninitialized. Whether it can happen in reality depends on whether ph-p_align can ever be 1. Next question -- what would the correct value for bdiff be in that case? 0? i.e. should the line be 'diff = bdiff = 0;'. Ken
Re: [PATCH?] Variable assignments...
On Mon, Jun 24, 2013 at 06:15:44PM +0200, Maxime Villard wrote:
Hi,
there are lots of useless assignment of variables in the code. I know this
kind of things does not really matter, but when I run my code scanner on
some parts of the source tree it gives me lots of them.
For example, for the net* directories:
== src/sys/net/if_bridge.c - l2017
u_int32_t cnt = 0;--- Here, we don't need to set cnt to 0
struct bridge_rtnode *n;
struct ifbareq bareq;
if (baconf-ifbac_len == 0)
onlycnt = 1;
for (i = 0, cnt = 0; i BRIDGE_RTABLE_SIZE; i++) --- set here
== src/sys/net/if_sppprubr.c - l403
int i = 0, x;
i = 0; Hum, hum, hum
== src/sys/net/if_pppx.c - l238
int rv = 0; --- ?
rv = rw_enter(pppx_devs_lk, RW_WRITE | RW_INTR);
== src/sys/netinet/if_output.c - l623
int transportmode = 0; - ?
transportmode = (tdb-tdb_dst.sa.sa_family == AF_INET)
(tdb-tdb_dst.sin.sin_addr.s_addr ==
ip-ip_dst.s_addr);
== src/sys/netinet6/raw_ip6.c - l380
int priv = 0; --
va_list ap;
int flags;
va_start(ap, m);
so = va_arg(ap, struct socket *);
dstsock = va_arg(ap, struct sockaddr_in6 *);
control = va_arg(ap, struct mbuf *);
va_end(ap);
in6p = sotoin6pcb(so);
priv = 0; --- ?
Same thing in several other places... Here is a patch for these dirs.
Ok/Comments?
Moving to a consistant style (I believe the current feeling is to
eliminate the declaration initialization) would be the best bet.
Ken
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.210
diff -u -r1.210 if_bridge.c
--- net/if_bridge.c 28 Mar 2013 23:10:05 - 1.210
+++ net/if_bridge.c 24 Jun 2013 15:55:08 -
@@ -2014,7 +2014,7 @@
bridge_rtfind(struct bridge_softc *sc, struct ifbaconf *baconf)
{
int i, error = 0, onlycnt = 0;
- u_int32_t cnt = 0;
+ u_int32_t cnt;
struct bridge_rtnode *n;
struct ifbareq bareq;
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.23
diff -u -r1.23 if_pppx.c
--- net/if_pppx.c 24 Jun 2013 09:34:59 - 1.23
+++ net/if_pppx.c 24 Jun 2013 15:55:08 -
@@ -235,7 +235,7 @@
pppxopen(dev_t dev, int flags, int mode, struct proc *p)
{
struct pppx_dev *pxd;
- int rv = 0;
+ int rv;
rv = rw_enter(pppx_devs_lk, RW_WRITE | RW_INTR);
if (rv != 0)
Index: net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.104
diff -u -r1.104 if_spppsubr.c
--- net/if_spppsubr.c 20 Jun 2013 12:03:40 - 1.104
+++ net/if_spppsubr.c 24 Jun 2013 15:55:09 -
@@ -4028,7 +4028,6 @@
STDDCL;
int i = 0, x;
- i = 0;
sp-rst_counter[IDX_CHAP] = sp-lcp.max_configure;
/*
Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.241
diff -u -r1.241 ip_output.c
--- netinet/ip_output.c 11 Jun 2013 18:15:53 - 1.241
+++ netinet/ip_output.c 24 Jun 2013 15:55:10 -
@@ -620,7 +620,7 @@
tdb-tdb_mtutimeout time_second) {
struct rtentry *rt = NULL;
int rt_mtucloned = 0;
- int transportmode = 0;
+ int transportmode;
transportmode = (tdb-tdb_dst.sa.sa_family == AF_INET)
(tdb-tdb_dst.sin.sin_addr.s_addr ==
Index: netinet6/raw_ip6.c
===
RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
retrieving revision 1.58
diff -u -r1.58 raw_ip6.c
--- netinet6/raw_ip6.c4 Jun 2013 19:11:52 - 1.58
+++ netinet6/raw_ip6.c24 Jun 2013 15:55:10 -
@@ -377,7 +377,6 @@
in6p = sotoin6pcb(so);
- priv = 0;
if ((so-so_state SS_PRIV) != 0)
priv = 1;
dst = dstsock-sin6_addr;
Re: Still More Secrets of Buffer Cache Enlargement.
On Sun, Jun 09, 2013 at 12:37:26PM -0600, Bob Beck wrote: Greetings all, Here's an up to date version of the buffer flipper that installs on post hackathon -current. This diff (~beck/viagra.diff15) contains one important change from the previous version - In the old cache, as buffers were never freed, we would put B_INVAL buffers in the cache at the head of the clean LRU. (B_INVAL buffers do not contain cachable data - so for example when a remove happens and a file's link count drops to 0, all it's buffers are marked B_INVAL). I noticed after some work with tedu at the end of the hackathon that we kept a lot of data in cache for removed files - it was because of this - and moving to the head of the LRU (behaviour that has been retained since the old static buffer cache) does not make sense with the modern dynamic one - so this diff has changed it to free the B_INVAL buffers right away instead of cacheing them. I'm running this on multiple arches and on my nfs servers feeding them. -Bob No issues so far! At 101% of last port (chromium) on bufferflipper crashing laptop. Ken
RFC 3442 (classless static routes) in dhclient
Anybody encountering dhcp environments that try to server out
classless static routes, i.e. dhcp option 121? Support for
static routes (option 33) thown in for free.
Apparently Microsoft Network Access Protection may be using them.
If so, tests of the diff below would be highly appreciated.
Ken
Index: clparse.c
===
RCS file: /cvs/src/sbin/dhclient/clparse.c,v
retrieving revision 1.57
diff -u -p -r1.57 clparse.c
--- clparse.c 2 May 2013 16:35:27 - 1.57
+++ clparse.c 2 Jun 2013 15:26:57 -
@@ -72,6 +72,9 @@ read_client_conf(void)
[config-requested_option_count++] = DHO_BROADCAST_ADDRESS;
config-requested_options
[config-requested_option_count++] = DHO_TIME_OFFSET;
+ /* RFC 3442 says CLASSLESS_STATIC_ROUTES must be before ROUTERS! */
+ config-requested_options
+ [config-requested_option_count++] = DHO_CLASSLESS_STATIC_ROUTES;
config-requested_options
[config-requested_option_count++] = DHO_ROUTERS;
config-requested_options
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.248
diff -u -p -r1.248 dhclient.c
--- dhclient.c 1 Jun 2013 16:26:07 - 1.248
+++ dhclient.c 2 Jun 2013 22:33:35 -
@@ -109,6 +109,8 @@ void socket_nonblockmode(int);
voidapply_ignore_list(char *);
void add_default_route(int, struct in_addr, struct in_addr);
+void add_static_routes(int, struct option_data *);
+void add_classless_static_routes(int, struct option_data *);
#defineROUNDUP(a) \
((a) 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
@@ -790,12 +792,21 @@ bind_lease(void)
* is done by the RTM_NEWADDR message being received.
*/
add_address(ifi-name, ifi-rdomain, client-new-address, mask);
- if (options[DHO_ROUTERS].len) {
- memset(gateway, 0, sizeof(gateway));
- /* XXX Only use FIRST router address for now. */
- memcpy(gateway.s_addr, options[DHO_ROUTERS].data,
- options[DHO_ROUTERS].len);
- add_default_route(ifi-rdomain, client-new-address, gateway);
+ if (options[DHO_CLASSLESS_STATIC_ROUTES].len) {
+ add_classless_static_routes(ifi-rdomain,
+ options[DHO_CLASSLESS_STATIC_ROUTES]);
+ } else {
+ if (options[DHO_ROUTERS].len) {
+ memset(gateway, 0, sizeof(gateway));
+ /* XXX Only use FIRST router address for now. */
+ memcpy(gateway.s_addr, options[DHO_ROUTERS].data,
+ options[DHO_ROUTERS].len);
+ add_default_route(ifi-rdomain, client-new-address,
+ gateway);
+ }
+ if (options[DHO_STATIC_ROUTES].len)
+ add_static_routes(ifi-rdomain,
+ options[DHO_STATIC_ROUTES]);
}
client-new-resolv_conf = resolv_conf_contents(
@@ -2280,27 +2291,76 @@ priv_write_file(struct imsg_write_file *
void
add_default_route(int rdomain, struct in_addr addr, struct in_addr gateway)
{
- struct imsg_add_routeimsg;
- int rslt;
-
- memset(imsg, 0, sizeof(imsg));
+ struct in_addr netmask;
+ int addrs;
- imsg.rdomain = rdomain;
- imsg.dest = addr;
- imsg.addrs = RTA_DST | RTA_NETMASK;
+ memset(netmask, 0, sizeof(netmask));
+ addrs = RTA_DST | RTA_NETMASK;
/*
* Set gateway address if and only if non-zero addr supplied. A
* gateway address of 0 implies '-iface'.
*/
- if (bcmp(gateway, addr, sizeof(addr)) != 0) {
- imsg.gateway = gateway;
- imsg.addrs |= RTA_GATEWAY;
+ if (bcmp(gateway, addr, sizeof(addr)) != 0)
+ addrs |= RTA_GATEWAY;
+
+ add_route(rdomain, addr, netmask, gateway, addrs);
+}
+
+void
+add_static_routes(int rdomain, struct option_data *static_routes)
+{
+ struct in_addr dest, netmask, gateway;
+ u_int8_t *addr;
+ int i;
+
+ memset(netmask, 0, sizeof(netmask)); /* Always 0 for class addrs. */
+
+ for (i = 0; (i + 7) static_routes-len; i += 8) {
+ addr = static_routes-data[i];
+ memset(dest, 0, sizeof(dest));
+ memset(gateway, 0, sizeof(gateway));
+
+ memcpy(dest.s_addr, addr, 4);
+ if (dest.s_addr == INADDR_ANY)
+ continue; /* RFC 2132 says 0.0.0.0 is not allowed. */
+ memcpy(gateway.s_addr, addr+4, 4);
+
+ /* XXX Order implies priority but we're ignoring that. */
+ add_route(rdomain, dest, netmask, gateway,
+
Re: Somewhat important ACPI diff
On Mon, May 20, 2013 at 06:57:56PM +0200, Mark Kettenis wrote: As diagnosed by some other people (armani@, jcs@?) a while ago, our code to deal with IndexField() operators in our AML interpreter is quite broken. It works for fields that are less than a byte in size, but anything else is pretty much completely busted. I wouldn't be surprised if this is the cause of many ACPI-related problems. One that comes to mind are the ridiculous temperatures reported by acpitz(4) that have been plaguing us since basically forever. I don't have a lot of machines that have AML with IndexField() operators. I've verified that those return the correct values, but this defenitely could use further testing on a wide variety of i386/amd64 hardware please. 6 amd64 boxes checked. 3 laptops, 3 non-laptops. intel procs (core and atom) amd procs. Various vintages, nothing more recent than 12 months or so. No regressions seen. Ken
Improve st(4) to make Bacula happier, 'modern' tapes faster
The diff below brings a bunch of improvements, mostly from Net/FreeBSD,
to the scsi tape driver st(4). In particular, running btape now
reports (for me) no errors no matter which combination of
Hardware End of Medium =
Fast Forward Space File =
settings are used. I'm told this should significantly improve the
speed of modern drives like LTO-3 for Bacula by allowing 'yes' for
both.
Tests sought to confirm/refute this, and of course spot any regressions
in other tape uses! Fixes/further improvements and comments always
welcome.
Ken
Index: scsi_all.h
===
RCS file: /cvs/src/sys/scsi/scsi_all.h,v
retrieving revision 1.53
diff -u -p -u -p -r1.53 scsi_all.h
--- scsi_all.h 8 Jul 2011 08:13:19 - 1.53
+++ scsi_all.h 11 May 2013 10:53:00 -
@@ -378,6 +378,11 @@ struct scsi_sense_data {
/* Additional sense code info */
#define ASC_ASCQ(ssd) ((ssd-add_sense_code 8) | ssd-add_sense_code_qual)
+#define SENSE_FILEMARK_DETECTED0x0001
+#define SENSE_END_OF_MEDIUM_DETECTED 0x0002
+#define SENSE_SETMARK_DETECTED 0x0003
+#define SENSE_BEGINNING_OF_MEDIUM_DETECTED 0x0004
+#define SENSE_END_OF_DATA_DETECTED 0x0005
#define SENSE_NOT_READY_BECOMING_READY 0x0401
#define SENSE_NOT_READY_INIT_REQUIRED 0x0402
#define SENSE_NOT_READY_FORMAT 0x0404
Index: st.c
===
RCS file: /cvs/src/sys/scsi/st.c,v
retrieving revision 1.121
diff -u -p -u -p -r1.121 st.c
--- st.c3 Jul 2011 15:47:18 - 1.121
+++ st.c12 May 2013 02:24:16 -
@@ -208,6 +208,7 @@ struct st_softc {
u_int32_t media_density;/* this is what it said when asked*/
int media_fileno; /* relative to BOT. -1 means unknown. */
int media_blkno;/* relative to BOF. -1 means unknown. */
+ int media_eom; /* relative to BOT. -1 means unknown. */
u_int drive_quirks; /* quirks of this drive */
@@ -265,19 +266,23 @@ struct cfdriver st_cd = {
#defineST_FIXEDBLOCKS 0x0008
#defineST_AT_FILEMARK 0x0010
#defineST_EIO_PENDING 0x0020 /* we couldn't report it then (had
data) */
+#defineST_EOM_PENDING 0x0040 /* we couldn't report it then (had
data) */
+#define ST_EOD_DETECTED0x0080
#defineST_FM_WRITTEN 0x0100 /*
* EOF file mark written -- used with
* ~ST_WRITTEN to indicate that multiple file
* marks have been written
*/
-#defineST_DYING0x40/* dying, when deactivated */
#defineST_BLANK_READ 0x0200 /* BLANK CHECK encountered already */
#defineST_2FM_AT_EOD 0x0400 /* write 2 file marks at EOD */
#defineST_MOUNTED 0x0800 /* Device is presently mounted */
#defineST_DONTBUFFER 0x1000 /* Disable buffering/caching */
#define ST_WAITING 0x2000
+#defineST_DYING0x4000 /* dying, when deactivated */
+#define ST_BOD_DETECTED0x8000
-#defineST_PER_ACTION (ST_AT_FILEMARK | ST_EIO_PENDING |
ST_BLANK_READ)
+#defineST_PER_ACTION (ST_AT_FILEMARK | ST_EIO_PENDING |
ST_EOM_PENDING | \
+ST_BLANK_READ)
#define stlookup(unit) (struct st_softc *)device_lookup(st_cd, (unit))
@@ -335,6 +340,7 @@ stattach(struct device *parent, struct d
/* Start up with media position unknown. */
st-media_fileno = -1;
st-media_blkno = -1;
+ st-media_eom = -1;
/*
* Reset the media loaded flag, sometimes the data
@@ -660,6 +666,9 @@ st_mount_tape(dev_t dev, int flags)
SCSI_IGNORE_ILLEGAL_REQUEST | SCSI_IGNORE_NOT_READY);
st-flags |= ST_MOUNTED;
sc_link-flags |= SDEV_MEDIA_LOADED;/* move earlier? */
+ st-media_fileno = 0;
+ st-media_blkno = 0;
+ st-media_eom = -1;
done:
device_unref(st-sc_dev);
@@ -927,7 +936,8 @@ ststart(struct scsi_xfer *xs)
}
/*
-* only FIXEDBLOCK devices have pending operations
+* Only FIXEDBLOCK devices have pending I/O or space
+* operations.
*/
if (st-flags ST_FIXEDBLOCKS) {
/*
@@ -962,26 +972,27 @@ ststart(struct scsi_xfer *xs)
continue; /* seek more work */
}
}
- /*
-* If we are at EIO (e.g. EOM) but have not reported it
-* yet then we should report it now
-*/
+ }
+
+ /*
+* If we are at EIO or EOM but
Re: rm(1) static addition
On Sat, Apr 27, 2013 at 09:12:21AM -0400, Eitan Adler wrote: On 27 April 2013 09:06, Kenneth R Westerback kwesterb...@rogers.com wrote: On Sat, Apr 27, 2013 at 08:10:41AM +0200, Otto Moerbeek wrote: On Sat, Apr 27, 2013 at 01:08:06AM -0400, Eitan Adler wrote: Hey all, Time for attempt #2! Adding static to internal function allows the compiler to better detect dead code (functions, variables, etc) and makes it easier for the compiler to optimize; e.g., since it knows a function will only called once it can inline code; or not output a symbol for a certain function. In general we don't lik this because it makes things harder to debug. For libraries, yes, but for programs, no. -Otto +1. We see way more 'nuke stupid static crap' diffs that 'add static' diffs. We are even dubious about almost all inline functions since they are also harder to debug and (on most 'modern' archs) add little if any performance but do make executables bigger. Just in case you have a 'use inline functions to speed things up just like XBSD' diff in the queue, and were about to hit another sensitive button issue. :-) Most of my diffs are take recent^W changes from the other BSDs if they are useful. FWIW I don't believe this sort of patch significantly affects debugging because that should be done with -O0 -g anyways. Odd how often people running release, and who don't want to compile shit, report problems we'd like them to be able to provide more info on. :-) Ken That said, thanks for the info. If I have other diffs which are more suitable to OpenBSD I'll be sure to send them. Most the remainder are similar cleanup or non-POSIX feature-adds. -- Eitan Adler
Re: rm(1) static addition
On Sat, Apr 27, 2013 at 08:10:41AM +0200, Otto Moerbeek wrote:
On Sat, Apr 27, 2013 at 01:08:06AM -0400, Eitan Adler wrote:
Hey all,
Time for attempt #2!
Adding static to internal function allows the compiler to better
detect dead code (functions, variables, etc) and makes it easier for
the compiler to optimize; e.g., since it knows a function will only
called once it can inline code; or not output a symbol for a certain
function.
In general we don't lik this because it makes things harder to debug.
For libraries, yes, but for programs, no.
-Otto
+1. We see way more 'nuke stupid static crap' diffs that 'add static'
diffs. We are even dubious about almost all inline functions since
they are also harder to debug and (on most 'modern' archs) add
little if any performance but do make executables bigger. Just in
case you have a 'use inline functions to speed things up just like
XBSD' diff in the queue, and were about to hit another sensitive
button issue. :-)
Ken
I'm offering this patch for review:
Index: rm.c
===
RCS file: /cvs/src/bin/rm/rm.c,v
retrieving revision 1.27
diff -u -r1.27 rm.c
--- rm.c5 Sep 2012 19:49:08 - 1.27
+++ rm.c27 Apr 2013 04:26:18 -
@@ -49,15 +49,15 @@
extern char *__progname;
-int dflag, eval, fflag, iflag, Pflag, stdin_ok;
+static int dflag, eval, fflag, iflag, Pflag, stdin_ok;
-intcheck(char *, char *, struct stat *);
-void checkdot(char **);
-void rm_file(char **);
-intrm_overwrite(char *, struct stat *);
-intpass(int, off_t, char *, size_t);
-void rm_tree(char **);
-void usage(void);
+static int check(char *, char *, struct stat *);
+static voidcheckdot(char **);
+static voidrm_file(char **);
+static int rm_overwrite(char *, struct stat *);
+static int pass(int, off_t, char *, size_t);
+static voidrm_tree(char **);
+static voidusage(void);
/*
* rm --
@@ -117,7 +117,7 @@
exit (eval);
}
-void
+static void
rm_tree(char **argv)
{
FTS *fts;
@@ -217,7 +217,7 @@
fts_close(fts);
}
-void
+static void
rm_file(char **argv)
{
struct stat sb;
@@ -271,7 +271,7 @@
* kernel support.
* Returns 1 for success.
*/
-int
+static int
rm_overwrite(char *file, struct stat *sbp)
{
struct stat sb, sb2;
@@ -324,7 +324,7 @@
return (0);
}
-int
+static int
pass(int fd, off_t len, char *buf, size_t bsize)
{
size_t wlen;
@@ -338,7 +338,7 @@
return (1);
}
-int
+static int
check(char *path, char *name, struct stat *sp)
{
int ch, first;
@@ -380,7 +380,7 @@
* trailing slashes have been removed, we'll remove them here.
*/
#define ISDOT(a) ((a)[0] == '.' (!(a)[1] || ((a)[1] == '.'
!(a)[2])))
-void
+static void
checkdot(char **argv)
{
char *p, **save, **t;
@@ -411,7 +411,7 @@
}
}
-void
+static void
usage(void)
{
(void)fprintf(stderr, usage: %s [-dfiPRr] file ...\n, __progname);
--
Eitan Adler
Re: tee rewrite
On Sun, Apr 21, 2013 at 01:33:55PM -0400, Ted Unangst wrote:
ok, it's not a rewrite, but I changed a lot of the lines.
Use better types, check errors against -1, delete some casts, stack
buffer eliminates one malloc, braces for long blocks.
As long as you're in there, why not eliminate LIST as well and just
use 'struct list'?
Ken
Index: tee.c
===
RCS file: /cvs/src/usr.bin/tee/tee.c,v
retrieving revision 1.7
diff -u -p -r1.7 tee.c
--- tee.c 27 Oct 2009 23:59:44 - 1.7
+++ tee.c 21 Apr 2013 07:27:16 -
@@ -42,30 +42,41 @@
#include locale.h
#include err.h
-typedef struct _list {
- struct _list *next;
+typedef struct list {
+ struct list *next;
int fd;
char *name;
} LIST;
LIST *head;
-void add(int, char *);
+static void
+add(int fd, char *name)
+{
+ LIST *p;
+
+ if ((p = malloc(sizeof(LIST))) == NULL)
+ err(1, NULL);
+ p-fd = fd;
+ p-name = name;
+ p-next = head;
+ head = p;
+}
int
main(int argc, char *argv[])
{
LIST *p;
- int n, fd, rval, wval;
+ int fd;
+ ssize_t n, rval, wval;
char *bp;
int append, ch, exitval;
- char *buf;
-#define BSIZE (8 * 1024)
+ char buf[8192];
setlocale(LC_ALL, );
append = 0;
- while ((ch = getopt(argc, argv, ai)) != -1)
- switch((char)ch) {
+ while ((ch = getopt(argc, argv, ai)) != -1) {
+ switch(ch) {
case 'a':
append = 1;
break;
@@ -77,23 +88,24 @@ main(int argc, char *argv[])
(void)fprintf(stderr, usage: tee [-ai] [file ...]\n);
exit(1);
}
+ }
argv += optind;
argc -= optind;
- if ((buf = malloc((size_t)BSIZE)) == NULL)
- err(1, NULL);
-
add(STDOUT_FILENO, stdout);
- for (exitval = 0; *argv; ++argv)
- if ((fd = open(*argv, append ? O_WRONLY|O_CREAT|O_APPEND :
- O_WRONLY|O_CREAT|O_TRUNC, DEFFILEMODE)) 0) {
+ exitval = 0;
+ while (*argv) {
+ if ((fd = open(*argv, O_WRONLY | O_CREAT |
+ (append ? O_APPEND : O_TRUNC), DEFFILEMODE)) == -1) {
warn(%s, *argv);
exitval = 1;
} else
add(fd, *argv);
+ argv++;
+ }
- while ((rval = read(STDIN_FILENO, buf, BSIZE)) 0)
+ while ((rval = read(STDIN_FILENO, buf, sizeof(buf))) 0) {
for (p = head; p; p = p-next) {
n = rval;
bp = buf;
@@ -106,7 +118,8 @@ main(int argc, char *argv[])
bp += wval;
} while (n -= wval);
}
- if (rval 0) {
+ }
+ if (rval == -1) {
warn(read);
exitval = 1;
}
@@ -119,17 +132,4 @@ main(int argc, char *argv[])
}
exit(exitval);
-}
-
-void
-add(int fd, char *name)
-{
- LIST *p;
-
- if ((p = malloc((size_t)sizeof(LIST))) == NULL)
- err(1, NULL);
- p-fd = fd;
- p-name = name;
- p-next = head;
- head = p;
}
Re: Another manpage grammar tweak (ath.4)
On Fri, Apr 12, 2013 at 07:40:05AM +0100, Jason McIntyre wrote: On Fri, Apr 12, 2013 at 08:30:16AM +0200, Alexander Hall wrote: .It AR5212 These devices support 802.11a, 802.11b, and 802.11g operation with transmit speeds as above for 802.11a, 802.11b, and 802.11g operation -(802.11g speeds are the same as for 802.11a speeds). +(802.11g speeds are the same as 802.11a speeds). .El .Pp All chips also support an Atheros Turbo Mode (TM) that operates in the hmm. i don;t think this is grammatically incorrect at all. it might sound strange to some ears, i guess. but wrong? why is it wrong? Cause the 802.11a speeds don't have speeds? Admittedly I'm not a native speaker, but I'd agree with the OP. it's not saying that. it says, in essence, that 11g speeds are the same as for (the) 11a speeds (listed above). it is omitting parts that can be left out because the intent should be fairly obvious. i guess we can reword it if folks think it sounds odd (or wrong ;) but if i had to do that, i'd say it'd sound better as the same as those for 802.11a. This sounds even better. My ok on that one if you feel you need it. :) i don;t really think it needs changed, unless folks are unhappy that it's unclear (or feel its wrong). i guess that's 2 votes so far to change it though ;) jmc I'm with jmc. Doesn't seem wrong to me, a native Canadian speaker. Ken
Re: goodbye to some isa devices
On Wed, Mar 27, 2013 at 08:14:20PM +, Creamy wrote: On Wed, Mar 27, 2013 at 08:05:47PM +, Miod Vallat wrote: In fact, to everybody else who is reading this, doesn't it just point out that 486 support is, effectively, already broken, (as I suspected), because the devices that typically go with machines of that era are suffering bit-rot in the tree? Absolutely not. First, 80486 support is not broken (but an FPU is required); You mis-understand, I am fully aware that the CPU itself is fully supported - my point was that it's likely that any 486 as a whole is more than likely to contain hardware that has issues which are going un-noticed because people are not using the code. second, isa drivers receiving few, if any, attention, doesn't mean they are no longer working. Where did I claim that, exactly? Ever heard of `if it ain't broke, don't touch it'? Well, maybe Alexey would have been happy for somebody to touch his SCSI driver and fix it, why don't you do it for him? Somebody broke it almost 20 releases ago, and guess what, from what I can gather it's still broken. So, if it IS broken, DO fix it. Or are you just trolling for the sake of it? I didn't expect that from you, frankly. Other people have been rude to me off-list, but I thought you were above that. Really, this community has an attitude problem - and you *need* more developers, believe me, you shouldn't be trying to scare them away. People who think we have an attitude problem self-evidently will be unlikely to fit in as developers. Or should we dissemble and surprise them with our attitude when they become developers? Because the attitude doesn't change much when one gets the magic decoder ring. Ken -- Creamy
Re: goodbye to some isa devices
On Tue, Mar 26, 2013 at 09:09:14AM -0400, Ted Unangst wrote: On Tue, Mar 26, 2013 at 11:13, Mark Kettenis wrote: Date: Tue, 26 Mar 2013 05:20:27 -0400 From: Ted Unangst t...@tedunangst.com These isa devs are already disabled and not particularly popular among our users. affected: tcic, sea, wds, eg, el The reason these devices are disabled is probably that their probe routines are destructive. So the fact that they are disabled doesn't necessarily mean that they don't work properly. I don't think maintaining these drivers is currently a huge burden on us. But decoupling them from the build will almost certainly lead to some degree of bitrot. Perfection is achieved when there's nothing left to take away. :) It's not so much that we spend time maintaining the source, but I do spend time compiling it. And I have to download it (3 times!) every time I install a new snapshot. Cumulatively, I've probably spent hours of my life waiting for these drivers' bits to go from here to there. I will selfishly claim that if I save five minutes of time this year by not compiling these files, that right there is more benefit than retaining support. I targeted disabled devices figuring they were least likely to be missed, but I honestly question the utility of any of these ISA network and SCSI drivers. They're going to be slow as shit. Besides, at this point, due to adding so many new drivers (kernel size has more than doubled in last ten years) the minimum RAM requirement is basically past ISA only machines. The segment of machines that lack PCI but support 32M or more of RAM is very narrow. And unlike sparc or vax, I don't think running OpenBSD on some ancient 486 is historically interesting. The ISA SCSI drivers have certainly received no love from me as a deliberate policy. This of course may be good or bad for their functionality. :-) And then there are the EISA SCSI drivers. Ken
Re: Add Soekris comBIOS detection to bios(4) on i386/amd64
On Tue, Mar 12, 2013 at 05:13:01AM -0400, Matt Dainty wrote:
* Matt Dainty m...@bodgit-n-scarper.com [2013-02-20 19:30:43]:
Attached are two patches for bios(4) on i386 amd64 that add support
for detecting the comBIOS on Soekris hardware, which then fills in the
hw.vendor hw.product sysctl variables as this hardware lacks any
SMBIOS to provide them. The idea is then these can be used in the
GPIO/LED driver for the net6501 I posted a while ago to simplify the
match logic.
Are there any objections to this being committed? It seems to work on
all Soekris boards. I can send the revised GPIO/LED driver for the
net6501.
Thanks
Matt
Putting in workarounds for particular vendors always puts my teeth on
edge. How many other bios weirdos will end up here?
Ken
--- sys/arch/amd64/amd64/bios.c.origTue Feb 19 01:56:56 2013
+++ sys/arch/amd64/amd64/bios.c Wed Feb 20 08:37:12 2013
@@ -95,6 +95,7 @@
vaddr_t va;
paddr_t pa, end;
u_int8_t *p;
+ int smbiosrev = 0;
/* see if we have SMBIOS extentions */
for (p = ISA_HOLE_VADDR(SMBIOS_START);
@@ -137,6 +138,10 @@
printf(: SMBIOS rev. %d.%d @ 0x%lx (%d entries),
hdr-majrev, hdr-minrev, hdr-addr, hdr-count);
+ smbiosrev = hdr-majrev * 100 + hdr-minrev;
+ if (hdr-minrev 10)
+ smbiosrev = hdr-majrev * 100 + hdr-minrev * 10;
+
bios.cookie = 0;
if (smbios_find_table(SMBIOS_TYPE_BIOS, bios)) {
sb = bios.tblhdr;
@@ -158,6 +163,39 @@
break;
}
printf(\n);
+
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (p = ISA_HOLE_VADDR(SMBIOS_START);
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); p++)
+ if (!memcmp(p, signature, strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT);
+ if (hw_vendor)
+ strlcpy(hw_vendor, signature,
+ strlen(signature) + 1);
+ p += strlen(signature);
+ break;
+ }
+
+ for (; hw_vendor
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END - 6); p++)
+ /*
+* Search only for net6501 in the comBIOS as that's
+* the only Soekris platform that can run amd64
+*/
+ if (!memcmp(p, net6501, 7)) {
+ hw_prod = malloc(8, M_DEVBUF, M_NOWAIT);
+ if (hw_prod) {
+ memcpy(hw_prod, p, 7);
+ hw_prod[7] = '\0';
+ }
+ break;
+ }
+ }
#if NACPI 0
{
--- sys/arch/i386/i386/bios.c.orig Tue Feb 19 06:36:42 2013
+++ sys/arch/i386/i386/bios.c Wed Feb 20 08:58:17 2013
@@ -330,6 +330,43 @@
printf(\n);
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!(flags BIOSF_SMBIOS) !smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (va = ISA_HOLE_VADDR(SMBIOS_START);
+ va = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); va++)
+ if (!memcmp((u_int8_t *)va, signature,
+ strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT);
+ if (hw_vendor)
+ strlcpy(hw_vendor, signature,
+ strlen(signature) + 1);
+ va += strlen(signature);
+ break;
+ }
+
+ for (; hw_vendor
+ va = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END - 6); va++)
+ /*
+* Search for net(4(5xx|801)|[56]501) which matches
+* the strings found in the comBIOS images
+*/
+ if (!memcmp((u_int8_t *)va, net45xx, 7) ||
+ !memcmp((u_int8_t *)va, net4801, 7) ||
+ !memcmp((u_int8_t *)va, net5501, 7) ||
+ !memcmp((u_int8_t *)va, net6501, 7)) {
+ hw_prod = malloc(8, M_DEVBUF, M_NOWAIT);
+ if (hw_prod) {
+ memcpy(hw_prod, (u_int8_t *)va, 7);
+
dhcpd ACK's too much
As reported by Andy via bugs@, our dhcpd is tad too accommodating
with its ACK'ing. According to RFC 2131 the server should only ACK
a REQUEST containing a server-identifier option if the server-identifier
identifies that server.
Andy confirms this works for him. Any other testers with challenging
dhcpd setups want to comment?
Ken
Index: dhcp.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dhcp.c,v
retrieving revision 1.33
diff -u -p -r1.33 dhcp.c
--- dhcp.c 14 Feb 2013 22:06:13 - 1.33
+++ dhcp.c 10 Mar 2013 15:16:27 -
@@ -321,6 +321,15 @@ dhcprequest(struct packet *packet)
return;
}
+ /*
+* Do not ACK a REQUEST intended for another server.
+*/
+ if (packet-options[DHO_DHCP_SERVER_IDENTIFIER].len == 4) {
+ if (memcmp(packet-options[DHO_DHCP_SERVER_IDENTIFIER].data,
+ packet-interface-primary_address, 4))
+ return;
+ }
+
/*
* If we own the lease that the client is asking for,
* and it's already been assigned to the client, ack it.
Re: Kill IFAFREE()
On Wed, Mar 06, 2013 at 03:58:22PM +0100, Mark Kettenis wrote:
Date: Wed, 6 Mar 2013 15:25:34 +0100
From: Martin Pieuchot mpieuc...@nolizard.org
On 05/03/13(Tue) 21:57, Claudio Jeker wrote:
On Tue, Mar 05, 2013 at 12:03:49PM +0100, Mike Belopuhov wrote:
On 5 March 2013 11:55, Mark Kettenis mark.kette...@xs4all.nl wrote:
Date: Tue, 5 Mar 2013 11:36:36 +0100
From: Martin Pieuchot mpieuc...@nolizard.org
The ifaddr structure contains a reference counter and two different
way
to check it before freeing its memory: a macro IFAFREE(), and a
function
ifafree().
Because the former calls the latter when the reference counter is
null,
and then also check for the reference counter, I see no point in
keeping
two ways to do the same thing.
Well, the point is probably that by doing the refcount check in the
macro you avoid a function call in most cases. It's very well
possible that this is a case of premature optimization. Almost
certainly the case unless the macro is called in a
performance-critical path. If it is called in a performance-critical
path, some benchmarking should probably be done to make sure this
doesn't impact something like packet forwarding performance in a
negative way.
to be fair, there are millions of these function calls. i highly
doubt one can measure any performance difference -- it'll all be
within error margin.
If we do IFAFREE() on a per packet basis then we do something wrong.
Glancing at the diff I see no hot pathes that would matter.
Exactly, we are unrefcounting address descriptors when detaching an
interface, removing addresses, modifying routes and obviously in some
ioctl() code paths.
So, ok?
I'm satisfied with the explanation. Premature optimisation is the
verdict and we all know that that's evil ;).
so ok kettenis@ (but you probably should get an ok from a real
networking person as well).
ok krw@ with same condition. :-)
Ken
diff --git sys/net/if.c sys/net/if.c
index 534d434..3edd0a7 100644
--- sys/net/if.c
+++ sys/net/if.c
@@ -599,7 +599,7 @@ do { \
continue;
ifa-ifa_ifp = NULL;
- IFAFREE(ifa);
+ ifafree(ifa);
}
for (ifg = TAILQ_FIRST(ifp-if_groups); ifg;
@@ -609,7 +609,7 @@ do { \
if_free_sadl(ifp);
ifnet_addrs[ifp-if_index]-ifa_ifp = NULL;
- IFAFREE(ifnet_addrs[ifp-if_index]);
+ ifafree(ifnet_addrs[ifp-if_index]);
ifnet_addrs[ifp-if_index] = NULL;
free(ifp-if_addrhooks, M_TEMP);
@@ -1007,7 +1007,7 @@ link_rtrequest(int cmd, struct rtentry *rt, struct
rt_addrinfo *info)
return;
if ((ifa = ifaof_ifpforaddr(dst, ifp)) != NULL) {
ifa-ifa_refcnt++;
- IFAFREE(rt-rt_ifa);
+ ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
if (ifa-ifa_rtrequest ifa-ifa_rtrequest != link_rtrequest)
ifa-ifa_rtrequest(cmd, rt, info);
@@ -1515,7 +1515,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data,
struct proc *p)
(struct in_ifaddr *)ifa, ia_list);
ifa_del(ifp, ifa);
ifa-ifa_ifp = NULL;
- IFAFREE(ifa);
+ ifafree(ifa);
}
#endif
splx(s);
diff --git sys/net/if.h sys/net/if.h
index 26ea6b1..27b209b 100644
--- sys/net/if.h
+++ sys/net/if.h
@@ -704,14 +704,6 @@ struct if_laddrreq {
#include net/if_arp.h
#ifdef _KERNEL
-#defineIFAFREE(ifa) \
-do { \
- if ((ifa)-ifa_refcnt = 0) \
- ifafree(ifa); \
- else \
- (ifa)-ifa_refcnt--; \
-} while (/* CONSTCOND */0)
-
#ifdef ALTQ
#defineIFQ_ENQUEUE(ifq, m, pattr, err)
\
diff --git sys/net/route.c sys/net/route.c
index 9ec8a47..a0dc710 100644
--- sys/net/route.c
+++ sys/net/route.c
@@ -401,7 +401,7 @@ rtfree(struct rtentry *rt)
rt_timer_remove_all(rt);
ifa = rt-rt_ifa;
if (ifa)
- IFAFREE(ifa);
+ ifafree(ifa);
rtlabel_unref(rt-rt_labelid);
#ifdef MPLS
if (rt-rt_flags RTF_MPLS)
@@ -926,7 +926,7 @@ rtrequest1(int req, struct rt_addrinfo *info, u_int8_t
prio,
if ((*ret_nrt)-rt_ifa-ifa_rtrequest)
(*ret_nrt)-rt_ifa-ifa_rtrequest(
RTM_DELETE, *ret_nrt, NULL);
- IFAFREE((*ret_nrt)-rt_ifa);
+ ifafree((*ret_nrt)-rt_ifa);
(*ret_nrt)-rt_ifa = ifa;
(*ret_nrt)-rt_ifp = ifa-ifa_ifp;
Re: out of memory errors seen on several AnonCVS servers
On Mon, Mar 04, 2013 at 11:13:22AM -0500, Ted Unangst wrote: On Mon, Mar 04, 2013 at 15:55, Stuart Henderson wrote: The client arch and software doesn't make a difference, the problem is on the server side. Problems seen when using opencvs server-side include giving out the wrong file version, and giving a checkout which can't reliably be used against a server running original CVS. hmmm, it's been a long time, but when I ran into this problem (like 2005), the client arch made all the difference. Ditto. Ken Or maybe back then the servers were all i386, too, and both client and server needed to be 32 bit? i.e., what used to be a client problem is now a server and client problem.
Re: install(1) confusing error message
On Thu, Feb 14, 2013 at 08:38:02PM +, Miod Vallat wrote:
This is what happens when install(1) is used to install files on a
read-only filesystem:
# mount -u -o ro /usr
# cd /usr/src
# make build
cd /usr/src/share/mk exec make install
install -c -o root -g bin -m 444 bsd.README bsd.dep.mk bsd.lib.mk bsd.man.mk
bsd.nls.mk bsd.obj.mk bsd.own.mk bsd.port.arch.mk bsd.port.mk
bsd.port.subdir.mk bsd.prog.mk bsd.regress.mk bsd.subdir.mk bsd.sys.mk
sys.mk bsd.lkm.mk bsd.xconf.mk bsd.xorg.mk /usr/share/mk
install: /usr/share/mk/bsd.README: File exists
*** Error 71 in share/mk (Makefile:13 'install')
*** Error 1 in /usr/src (Makefile:78 'build')
#
Not really helpful. With the diff below, I will now get:
install: /usr/share/mk/bsd.README: Read-only file system
which helps figuring out what is wrong.
Miod
Index: xinstall.c
===
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.52
diff -u -p -r1.52 xinstall.c
--- xinstall.c14 Sep 2012 00:00:29 - 1.52
+++ xinstall.c14 Feb 2013 20:32:21 -
@@ -639,8 +639,10 @@ create_newfile(char *path, struct stat *
/* It is ok for the target file not to exist. */
if (rename(path, backup) 0 errno != ENOENT)
err(EX_OSERR, rename: %s to %s (errno %d), path,
backup, errno);
- } else
- (void)unlink(path);
+ } else {
+ if (unlink(path) 0 errno != ENOENT)
+ err(EX_OSERR, %s, path);
+ }
return(open(path, O_CREAT | O_RDWR | O_EXCL, S_IRUSR | S_IWUSR));
}
Makes sense to me.
Ken
dhclient massive update -- test now or forever hold your peace!
As advertised a few months ago, dhclient(8) have been substantially reworked. The functional changes should all be in -current and snapshots dated later than today. Workarounds for most reported uses of dhclient-script have been found. Now is the time to test dhclient hard to discover if/how your setup will break. Or it will be shipping as is in 5.3! Ken
Re: diff: dhcp-options(5) vs. dhcpd/tables.c
On Thu, Jan 03, 2013 at 04:00:38PM +0100, MERIGHI Marcus wrote: j...@kerhand.co.uk (Jason McIntyre), 2013.01.02 (Wed) 18:39 (CET): On Sun, Dec 16, 2012 at 07:24:53PM +0100, MERIGHI Marcus wrote: playing with option-252 I found it already has a name. Found that as well: http://marc.info/?l=openbsd-miscm=126573542214773 Index: dhcp-options.5 === fixed now by krw. jmc I find it desireable to have the option numbers mentioned in dhcp-options(5). Rationale: - the names assigned are arbitrary, the numbers are RFC'd. - searching the interwebs often returns the numbers and an arbitrary name that doesn't necessarily help in finding the name used by OpenBSDs dhcpd(8). - having to look up number-to-name in usr.sbin/dhcpd/tables.c is not intended, I suppose. That is going to be a lot of work, therefore asking in advance: any interest in such a diff? current: option arp-cache-timeout uint32; This option specifies the timeout in seconds for ARP cache entries. suggestion: option arp-cache-timeout uint32; This option specifies the timeout in seconds for ARP cache entries. option-35, RFC 2132, Section 6.2. Additionally, as something that I cannot code myself, I would like to be allowed to specify ``option-35 3600;'' even if there's a name assigned to that number. Bye, Marcus I wouldn't mind seeing this information, but I don't think adding such a reference to each section is the best way. I was thinking a table with columns Option#, Option Name, Relevant RFC(s). Then a bunch of such references in each section could be removed. Ken
Re: change if_iqdrops to if_ierrors
On Thu, Nov 29, 2012 at 04:41:09PM +0100, Mike Belopuhov wrote:
hi,
drivers ex age alc ale jme se vic vte xe upl and octeon/cmac
make use of the if_iqdrops counter that is not shown by any of our
tools (like netstat). looks like most of its usage comes from
freebsd where they show it in the netstat -di output in a new
column. do we want to do that or just convert them to if_ierrors
since 90% of our drivers do only if_ierrors. there's also doesn't
seem to be any rule when to use if_iqdrops (well, since in most
drivers there's no input queueing -- check out upl(4) :)
the diff below changes all the drivers in our tree to use
if_ierrors instead of if_iqdrops. i've decided to leave
octeon/cmac driver as is because if_iqdrops is used for
debugging purposes there.
ack? nack? meh?
Seems like a good idea to me to not lose those errors. I have
no great desire for a new column in netstat, but no great
antagonism to one either.
Ken
diff --git sys/dev/isa/if_ex.c sys/dev/isa/if_ex.c
index 49eb077..f93165f 100644
--- sys/dev/isa/if_ex.c
+++ sys/dev/isa/if_ex.c
@@ -661,7 +661,7 @@ ex_rx_intr(struct ex_softc *sc)
MGETHDR(m, M_DONTWAIT, MT_DATA);
ipkt = m;
if (ipkt == NULL)
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
else {
ipkt-m_pkthdr.rcvif = ifp;
ipkt-m_pkthdr.len = pkt_len;
@@ -673,7 +673,7 @@ ex_rx_intr(struct ex_softc *sc)
m-m_len = MCLBYTES;
else {
m_freem(ipkt);
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
goto rx_another;
}
}
@@ -696,7 +696,7 @@ ex_rx_intr(struct ex_softc *sc)
MT_DATA);
if (m-m_next == NULL) {
m_freem(ipkt);
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
goto rx_another;
}
m = m-m_next;
diff --git sys/dev/pci/if_age.c sys/dev/pci/if_age.c
index f5f6de5..53903c1 100644
--- sys/dev/pci/if_age.c
+++ sys/dev/pci/if_age.c
@@ -1329,7 +1329,7 @@ age_rxeof(struct age_softc *sc, struct rx_rdesc *rxrd)
desc = rxd-rx_desc;
/* Add a new receive buffer to the ring. */
if (age_newbuf(sc, rxd) != 0) {
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
/* Reuse Rx buffers. */
if (sc-age_cdata.age_rxhead != NULL) {
m_freem(sc-age_cdata.age_rxhead);
diff --git sys/dev/pci/if_alc.c sys/dev/pci/if_alc.c
index eac4148..ef22bb5 100644
--- sys/dev/pci/if_alc.c
+++ sys/dev/pci/if_alc.c
@@ -1952,7 +1952,7 @@ alc_rxeof(struct alc_softc *sc, struct rx_rdesc *rrd)
mp = rxd-rx_m;
/* Add a new receive buffer to the ring. */
if (alc_newbuf(sc, rxd) != 0) {
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
/* Reuse Rx buffers. */
if (sc-alc_cdata.alc_rxhead != NULL)
m_freem(sc-alc_cdata.alc_rxhead);
diff --git sys/dev/pci/if_ale.c sys/dev/pci/if_ale.c
index 1e5004e..ef6348f 100644
--- sys/dev/pci/if_ale.c
+++ sys/dev/pci/if_ale.c
@@ -1552,7 +1552,7 @@ ale_rxeof(struct ale_softc *sc)
m = m_devget((char *)(rs + 1), length - ETHER_CRC_LEN,
ETHER_ALIGN, ifp, NULL);
if (m == NULL) {
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
ale_rx_update_page(sc, rx_page, length, prod);
continue;
}
diff --git sys/dev/pci/if_jme.c sys/dev/pci/if_jme.c
index 25c954f..37a1efa 100644
--- sys/dev/pci/if_jme.c
+++ sys/dev/pci/if_jme.c
@@ -1602,7 +1602,7 @@ jme_rxpkt(struct jme_softc *sc)
/* Add a new receive buffer to the ring. */
if (jme_newbuf(sc, rxd) != 0) {
- ifp-if_iqdrops++;
+ ifp-if_ierrors++;
/* Reuse buffer. */
jme_discard_rxbufs(sc, cons, nsegs - count);
if (sc-jme_cdata.jme_rxhead != NULL) {
diff --git sys/dev/pci/if_se.c sys/dev/pci/if_se.c
index
Re: hostname.if(5) clarification
On Mon, Nov 26, 2012 at 04:26:12PM +, Jason McIntyre wrote: On Mon, Nov 26, 2012 at 04:30:47PM +0200, Paul Irofti wrote: Be more specific about the order of interpretation. Okay? diff --git share/man/man5/hostname.if.5 share/man/man5/hostname.if.5 index b07459f..aa8446f 100644 --- share/man/man5/hostname.if.5 +++ share/man/man5/hostname.if.5 @@ -49,6 +49,8 @@ A configuration file is not needed for lo0. The configuration information is expressed in a line-by-line packed format which makes the most common cases simpler; those dense formats are described below. +The order of the configuration lines matters, they are interpreted from the +top down. Any lines not matching these packed formats are passed directly to .Xr ifconfig 8 . The packed formats are converted using a somewhat inflexible parser and if we say this, then we should provide guidance to folks about how to order the lines. what is the specific problem, or the general rule, that you are addressing? jmc I suggest the following. I think the dhcp example is likely a common use case that is obvious and clear. Ken Index: hostname.if.5 === RCS file: /cvs/src/share/man/man5/hostname.if.5,v retrieving revision 1.56 diff -u -p -r1.56 hostname.if.5 --- hostname.if.5 8 Jul 2011 01:30:26 - 1.56 +++ hostname.if.5 26 Nov 2012 17:29:13 - @@ -63,6 +63,17 @@ For example: .Bd -literal -offset indent inet 10.0.0.1 255.255.255.0 10.0.0.255 description Bob's uplink .Ed +.Pp +Each line is processed separately and in order. +For example: +.Bd -literal -offset indent +nwid mynwid +wpakey mywpakey +dhcp +.Ed +.Pp +would run ifconfig to set the nwid of the interface, run it again to set the wpakey of the interface, and then start +.Xr dhclient 8 . .Sh STATIC ADDRESS CONFIGURATION The following packed formats are valid for configuring network interfaces with static addresses:
Re: hostname.if(5) clarification
On Mon, Nov 26, 2012 at 05:40:06PM +, Jason McIntyre wrote: On Mon, Nov 26, 2012 at 07:19:23PM +0200, Paul Irofti wrote: On Mon, Nov 26, 2012 at 04:26:12PM +, Jason McIntyre wrote: On Mon, Nov 26, 2012 at 04:30:47PM +0200, Paul Irofti wrote: Be more specific about the order of interpretation. Okay? diff --git share/man/man5/hostname.if.5 share/man/man5/hostname.if.5 index b07459f..aa8446f 100644 --- share/man/man5/hostname.if.5 +++ share/man/man5/hostname.if.5 @@ -49,6 +49,8 @@ A configuration file is not needed for lo0. The configuration information is expressed in a line-by-line packed format which makes the most common cases simpler; those dense formats are described below. +The order of the configuration lines matters, they are interpreted from the +top down. Any lines not matching these packed formats are passed directly to .Xr ifconfig 8 . The packed formats are converted using a somewhat inflexible parser and if we say this, then we should provide guidance to folks about how to order the lines. what is the specific problem, or the general rule, that you are addressing? Problem: /etc/hostname.iwn0: dhcp nwid foo wpakey bar Gets neighbour's lease then drops it then gets the lease from the foo network using the bar wpakey. Solution: /etc/hostname.iwn0 nwid foo wpakey bar dhcp Sets the network to foo and associates a password to it and then tries to get a lease. Order matters. Perhaps there's a better way to phrase it but, as far as guidance goes, I guess it's not quite possible to do that because ifconfig alone has a plethora of possible usages. does dhcp nwid foo wpakey bar give you problems too? because hostname.if(5) suggests it should not: A DHCP-configured network interface setup consists of dhcp options There have been problems reported with doing everything on one line in the past. so if it isn;t working, isn;t that indicative of a worse problem? or that we have not documented how dhcp works sufficiently? Not sure how much more we can document here. I'm actually wondering if it wouldn't be more clear to eliminate the 'options' processing after 'dhcp', i.e. make people do those things on separate, preceeding lines. we can;t just say order matters, but not provide any guidance. having said that, i think the text The packed formats are converted, which i think deraadt added, was meant to address something like this. maybe he remembers? Well, hostname.if is simply a mechanism to script ifconfig invocations. If you don't know in what order you need to issue the ifconfig invocations required to configure your network, I'm not sure if hostname.if can explain it in a reasonable amount of space. anyway...i still dislike the idea of just saying order matters. also, could someone really expect the file to not be parsed top down (i don;t know, i'm just asking. it seems unlikely to me you'd start parsing from the end and work up)? jmc The misunderstanding I have seen run along the lines that all the lines will be processed and then the system will issue a coherent set of commands to achieve the described network. When really it is, as I said, just a way to put all the ifconfig and related commands in one file. . Ken
Re: athn(4) resets entire chip when switching channels
On Sat, Oct 20, 2012 at 09:16:38PM +0200, Stefan Sperling wrote: This looks like an obvious and accidental coding error. But I have no working athn(4) hardware to confirm that fixing it doesn't do any harm. Can athn(4) users please test this? Thanks. Index: athn.c === RCS file: /cvs/src/sys/dev/ic/athn.c,v retrieving revision 1.74 diff -u -p -r1.74 athn.c --- athn.c20 Oct 2012 09:54:20 - 1.74 +++ athn.c20 Oct 2012 19:07:52 - @@ -915,8 +915,8 @@ athn_switch_chan(struct athn_softc *sc, #ifdef notyet /* AR9280 needs a full reset. */ if (AR_SREV_9280(sc)) -#endif goto reset; +#endif /* If band or bandwidth changes, we need to do a full reset. */ if (c-ic_flags != sc-curchan-ic_flags || If anything the athn0 in my Acer Aspire 1551 works better. Along with the kill switch fix, it now seems to attach quicker to the crappy net at the hospital and I can now reboot the box without losing the athn. My athn is athn0 at pci3 dev 0 function 0 Atheros AR9287 rev 0x01: apic 2 int 17 athn0: AR9287 rev 2 (2T2R), ROM rev 4, address ec:55:f9:3e:18:fa Ken
Re: update athn(4) ar9485 initvals (please test on any athn(4))
On Sat, Oct 13, 2012 at 07:26:30PM +0200, Stefan Sperling wrote:
On Sun, Oct 07, 2012 at 06:24:39PM +0200, Stefan Sperling wrote:
The init values athn(4) has for the ar9485 are for version 1.0 of
this chip, which according to Atheros Linux developers was never sold:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=903946e6e21ef4dd678acafb8881cabde9182caf
This diff updates the initvals to what the Linux driver is using
for the 1.1 generation of the ar9485.
Because the serdes values are written to different registers on the
ar9485 I had to tweak code for other athn(4) devices, too. So please
test this on any athn(4) to make sure there are no regressions. Thanks!
krw@ reported that the prior diff crashed his machine because I
overlooked the serdes values table for non-pcie devices.
Here's a hopefully fixed diff. Again, please test for regressions.
This one does *not* blow up athn0 machine. :-)
No regressions noted so far.
Ken
Index: ar5416reg.h
===
RCS file: /cvs/src/sys/dev/ic/ar5416reg.h,v
retrieving revision 1.4
diff -u -p -r1.4 ar5416reg.h
--- ar5416reg.h 10 Jun 2012 21:23:36 - 1.4
+++ ar5416reg.h 13 Oct 2012 17:20:50 -
@@ -811,6 +811,20 @@ static const uint32_t ar5416_bank6_vals[
/*
* Serializer/Deserializer programming.
*/
+
+static const uint32_t ar5416_serdes_regs[] = {
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES2
+};
+
static const uint32_t ar5416_serdes_vals[] = {
0x9248fc00,
0x24924924,
@@ -821,10 +835,12 @@ static const uint32_t ar5416_serdes_vals
0x001defff,
0x1aaabe40,
0xbe105554,
- 0x000e3007
+ 0x000e3007,
+ 0x
};
static const struct athn_serdes ar5416_serdes = {
nitems(ar5416_serdes_vals),
- ar5416_serdes_vals
+ ar5416_serdes_regs,
+ ar5416_serdes_vals,
};
Index: ar9280reg.h
===
RCS file: /cvs/src/sys/dev/ic/ar9280reg.h,v
retrieving revision 1.5
diff -u -p -r1.5 ar9280reg.h
--- ar9280reg.h 10 Jun 2012 21:23:36 - 1.5
+++ ar9280reg.h 13 Oct 2012 17:20:50 -
@@ -586,6 +586,20 @@ static const struct athn_gain ar9280_2_0
/*
* Serializer/Deserializer programming.
*/
+
+static const uint32_t ar9280_2_0_serdes_regs[] = {
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES,
+ AR_PCIE_SERDES2,
+};
+
static const uint32_t ar9280_2_0_serdes_vals[] = {
0x9248fd00,
0x24924924,
@@ -599,10 +613,12 @@ static const uint32_t ar9280_2_0_serdes_
#endif
0x1aaabe41,
0xbe105554,
- 0x00043007
+ 0x00043007,
+ 0x
};
static const struct athn_serdes ar9280_2_0_serdes = {
nitems(ar9280_2_0_serdes_vals),
+ ar9280_2_0_serdes_regs,
ar9280_2_0_serdes_vals
};
Index: ar9380.c
===
RCS file: /cvs/src/sys/dev/ic/ar9380.c,v
retrieving revision 1.15
diff -u -p -r1.15 ar9380.c
--- ar9380.c 10 Jun 2012 21:23:36 - 1.15
+++ ar9380.c 13 Oct 2012 17:20:50 -
@@ -117,11 +117,13 @@ ar9380_attach(struct athn_softc *sc)
sc-cca_max_2g = AR9380_PHY_CCA_MAX_GOOD_VAL_2GHZ;
sc-cca_min_5g = AR9380_PHY_CCA_MIN_GOOD_VAL_5GHZ;
sc-cca_max_5g = AR9380_PHY_CCA_MAX_GOOD_VAL_5GHZ;
- if (AR_SREV_9485(sc))
- sc-ini = ar9485_1_0_ini;
- else
+ if (AR_SREV_9485(sc)) {
+ sc-ini = ar9485_1_1_ini;
+ sc-serdes = ar9485_1_1_serdes;
+ } else {
sc-ini = ar9380_2_2_ini;
- sc-serdes = ar9380_2_2_serdes;
+ sc-serdes = ar9380_2_2_serdes;
+ }
return (ar9003_attach(sc));
}
@@ -179,7 +181,7 @@ ar9380_setup(struct athn_softc *sc)
else
sc-rx_gain = ar9380_2_2_rx_gain;
} else
- sc-rx_gain = ar9485_1_0_rx_gain;
+ sc-rx_gain = ar9485_1_1_rx_gain;
/* Select initialization values based on ROM. */
type = MS(eep-baseEepHeader.txrxgain, AR_EEP_TX_GAIN);
@@ -193,7 +195,7 @@ ar9380_setup(struct athn_softc *sc)
else
sc-tx_gain = ar9380_2_2_tx_gain;
} else
- sc-tx_gain = ar9485_1_0_tx_gain;
+ sc-tx_gain = ar9485_1_1_tx_gain;
}
const uint8_t *
Index: ar9380reg.h
===
RCS file: /cvs/src/sys/dev/ic/ar9380reg.h,v
retrieving revision 1.17
diff -u -p -r1.17
Re: Unable to mount SAN IBM DS3400 through Qlogic HBA 24XX on openbsd 5.1
On Thu, Oct 11, 2012 at 03:25:26PM +0530, mu...@nitrkl.ac.in wrote: Still i am waiting for some hope. Nobody attach SAN With openbsd till now in the world ?. As we pointed out earlier, the isp 2400 4Gbit series cards are not yet supported by OpenBSD. Ken Dear all, I am unable to detect and mount SAN volume. Can anyone guide me about the step or tutorial. How to detect/mount SAN VOLUME. Here i am attaching the dmesg and error given below isp0 at pci4 dev 0 function 0 QLogic ISP2432 rev 0x03: apic 9 int 6 isp0: board type 2422 rev 0x3, resident firmware rev 4.0.26 scsibus0 at isp0: 512 targets, WWPN 2124ff364ae8, WWNN 2024ff364ae8 isp0: 0.0.0 FCP RESPONSE: 0x2 isp0: 0.0.0 FCP RESPONSE: 0x2 isp0: 0.1.0 FCP RESPONSE: 0x2 isp0: 0.1.0 FCP RESPONSE: 0x2 isp1 at pci4 dev 0 function 1 QLogic ISP2432 rev 0x03: apic 9 int 13 isp1: board type 2422 rev 0x3, resident firmware rev 4.0.26 scsibus1 at isp1: 512 targets, WWPN 2124ff364ae9, WWNN 2024ff364ae9 isp1: 0.0.0 FCP RESPONSE: 0x2 isp1: 0.0.0 FCP RESPONSE: 0x2 isp1: 0.1.0 FCP RESPONSE: 0x2 isp1: 0.1.0 FCP RESPONSE: 0x2 Any help will be appreciable. Best Regards - This email was sent using the NIT Rourkela Webmail. Nation Institute of Technology - Rourkela http://www.nitrkl.ac.in This mail has been scanned by Cyberoam based UTM at NIT, Rourkela. If your mail still contains virus forward it to s...@nitrkl.ac.in
Re: ral rt2661 tx interrupt race fix
On Thu, Oct 04, 2012 at 07:21:50PM +0200, Stefan Sperling wrote: On Sun, Sep 30, 2012 at 10:32:23PM +0100, Tom Murphy wrote: Stefan, Your patch works well on my system: ral0 at pci0 dev 14 function 0 Ralink RT2661 rev 0x00: irq 10, address 00:14:85:d5:39:bb ral0: MAC/BBP RT2661D, RF RT2529 (MIMO XR) Only problem is downloading from the net is extremely slow. Benchmarks have it at 512 KB/sec as opposed to 10 megabits/s. This is (internet) - OpenBSD - ral0 - wifi client But it doesn't crash or bring up OACTIVE flag anymore which is fantastic.. however, it's a little to slow to use with any regularity. Uploads are fine (wifi - ral(4) - OpenBSD - out to the net). I've already replied to Tom in private requesting some additional testing but I would still be interested in other reports about transmission speed issues with ral. I've also noticed that my ral-based AP can be ridiculously slow. I believe this is a separate bug which is independent of the OACTIVE lock-up problem. Is anyone else out there seeing this, too? IIRC dragonfly have some ral performance fixes in their git history which haven't been ported back to OpenBSD yet. I gave up on my ral ap a couple of years ago due to ridiculous slowness, but the athn replacement was just as slow. Got a new motherboard going into the firewall shortly and may be motivated to recheck the wireless performance of -current then. Ken
Re: compile kernel with isp qlogic
On Mon, Oct 01, 2012 at 06:32:43PM +0530, mohit sah wrote: Can any one tell me the right way to compile the kernel with isp. -- Mohit Sah isp(4) is compiled into GENERIC on alpha, amd64, hppa, i386, macppc, sparc, and sparc64. What is the problem you are encountering? Ken
Re: gem(4): simplify gem_attach_pci() variant detection code a bit
On Fri, Sep 28, 2012 at 09:31:34AM +0200, Christiano F. Haesbaert wrote:
On Fri, Sep 28, 2012 at 02:42:18AM -0400, Brad Smith wrote:
On Wed, Sep 26, 2012 at 03:32:37PM -0400, Brad Smith wrote:
Simplify the gem(4) variant detection code a bit.
OK?
How about this..
Index: if_gem_pci.c
===
RCS file: /home/cvs/src/sys/dev/pci/if_gem_pci.c,v
retrieving revision 1.32
diff -u -p -r1.32 if_gem_pci.c
--- if_gem_pci.c3 Apr 2011 15:36:02 - 1.32
+++ if_gem_pci.c28 Sep 2012 05:16:00 -
@@ -227,22 +227,19 @@ gem_attach_pci(struct device *parent, st
sc-sc_pci = 1; /* X should all be done in bus_dma. */
- if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_GEMNETWORK)
+ switch (PCI_PRODUCT(pa-pa_id)) {
+ case PCI_PRODUCT_SUN_GEMNETWORK:
sc-sc_variant = GEM_SUN_GEM;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_ERINETWORK)
+ break;
+ case PCI_PRODUCT_SUN_ERINETWORK:
sc-sc_variant = GEM_SUN_ERI;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_K2_GMAC)
+ break;
+ case PCI_PRODUCT_APPLE_K2_GMAC:
sc-sc_variant = GEM_APPLE_K2_GMAC;
+ break;
+ default:
+ sc-sc_variant = GEM_APPLE_GMAC;
+ }
#define PCI_GEM_BASEADDR 0x10
if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,
Ok by me, but when I said acknowledge I meant this, I'm ok with either,
if kettenis doesn't mind :=).
Index: if_gem_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_gem_pci.c,v
retrieving revision 1.32
diff -d -u -p -r1.32 if_gem_pci.c
--- if_gem_pci.c 3 Apr 2011 15:36:02 - 1.32
+++ if_gem_pci.c 28 Sep 2012 07:26:23 -
@@ -227,22 +227,27 @@ gem_attach_pci(struct device *parent, st
sc-sc_pci = 1; /* X should all be done in bus_dma. */
- if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_GEMNETWORK)
+ switch (PCI_PRODUCT(pa-pa_id)) {
+ case PCI_PRODUCT_SUN_GEMNETWORK:
sc-sc_variant = GEM_SUN_GEM;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_SUN_ERINETWORK)
+ break;
+ case PCI_PRODUCT_SUN_ERINETWORK:
sc-sc_variant = GEM_SUN_ERI;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_INTREPID2_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_PANGEA_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_SHASTA_GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTHGMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_UNINORTH2GMAC)
- sc-sc_variant = GEM_APPLE_GMAC;
- else if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_APPLE_K2_GMAC)
+ break;
+ case PCI_PRODUCT_APPLE_K2_GMAC:
sc-sc_variant = GEM_APPLE_K2_GMAC;
+ break;
+ case PCI_PRODUCT_APPLE_INTREPID2_GMAC:
+ case PCI_PRODUCT_APPLE_PANGEA_GMAC:
+ case PCI_PRODUCT_APPLE_SHASTA_GMAC:
+ case PCI_PRODUCT_APPLE_UNINORTHGMAC:
+ case PCI_PRODUCT_APPLE_UNINORTH2GMAC:
+ sc-sc_variant = GEM_APPLE_GMAC;
+ break;
+ default:
+ printf(: unknown variant 0x%x\n, sc-sc_variant);
+ return;
+ }
#define PCI_GEM_BASEADDR 0x10
if (pci_mapreg_map(pa, PCI_GEM_BASEADDR, type, 0,
This is better IMHO. I was mildly concerned about the previous versions that
seemed to recognize previously unknown chips. ok krw@
Ken
Re: make -j and errors
On Wed, Sep 26, 2012 at 06:21:34PM +0200, Marc Espie wrote: I've been thinking some more about it. POSIX says very little about parallel makes. The more I think about it, the more I think gnu-make's approach on this is stupid: if a job errors out in a fatal way, what do we gain if we keep going ? Especially for high -j values, the quicker we die, the better, as far as the error is concerned. (note that, in sequential mode, the first fatal error will kill us, so there's no point in considering further stuff running). but what about commands that take a long time to run ? Well, make already has a standard mechanism to flag those, that's called .PRECIOUS So, instead of the -dq quick death debugging option, I suggest we move to the following semantics: in case of an error, send ^C to all jobs making targets that are not tagged .PRECIOUS, wait for everything to come back, and that's it... Sounds like a rational approach to me. Ken
Re: hook-up acpi locking
On Wed, Sep 19, 2012 at 12:22:49AM +0300, Paul Irofti wrote:
Any reason we have this disabled?
I ran with this diff in for quite some time w/o any problems.
Can you test this and let me know if anything bad happens?
I'd like to enable this codepath and I have diffs depending on it.
Index: dev/acpi/dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.197
diff -u -p -r1.197 dsdt.c
--- dev/acpi/dsdt.c 16 Jul 2012 15:27:11 - 1.197
+++ dev/acpi/dsdt.c 18 Sep 2012 21:20:30 -
@@ -729,8 +729,6 @@ void aml_lockfield(struct aml_scope *, s
long acpi_acquire_global_lock(void*);
long acpi_release_global_lock(void*);
static long global_lock_count = 0;
-#define acpi_acquire_global_lock(x) 1
-#define acpi_release_global_lock(x) 0
void
aml_lockfield(struct aml_scope *scope, struct aml_value *field)
{
Not bad happens on my Aspire. But then the Aspire has never been
able to suspend more than once between power cycles and even then
only from X. :-).
Ken
Re: msdosfs_vnops.c u_char toname diff [Was: Re: panic: smashed stack in msdosfs_rename]
On Tue, Sep 04, 2012 at 02:56:40PM +0200, MERIGHI Marcus wrote:
with the diff below my ``panic: smashed stack in msdosfs_rename''
problem does not appear any more.
Index: msdosfs_vnops.c
===
RCS file: /cvs/src/sys/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.82
diff -u -r1.82 msdosfs_vnops.c
--- msdosfs_vnops.c 11 Jul 2012 12:39:20 - 1.82
+++ msdosfs_vnops.c 4 Sep 2012 09:28:32 -
@@ -860,7 +860,7 @@
struct componentname *fcnp = ap-a_fcnp;
struct proc *p = curproc; /* XXX */
struct denode *ip, *xp, *dp, *zp;
- u_char toname[11], oldname[11];
+ u_char toname[12], oldname[11];
uint32_t from_diroffset, to_diroffset;
u_char to_count;
int doingdirectory = 0, newparent = 0;
below is my lengthy report to bugs@ with some explanation.
Bye, Marcus
The problem seems to be rooted in a desire to printf() the dosname
in a debug statement. Otherwise the dos file names are not treated
as strings anywhere.
An alternate solution, that restores the symmetry between unix2dosfn()
and dos2unixfn(), is to use %.11s to print the dos file name in that
debug chunk, and otherwise consistantly treat the various dos file
names as 11-byte arrays.
Eliminiating one magic number (12) would seem a good thing. :-)
Comments about the non-stringiness of these vars might be good too.
Ken
Index: msdosfs_conv.c
===
RCS file: /cvs/src/sys/msdosfs/msdosfs_conv.c,v
retrieving revision 1.14
diff -u -p -r1.14 msdosfs_conv.c
--- msdosfs_conv.c 13 Aug 2009 22:34:29 - 1.14
+++ msdosfs_conv.c 4 Sep 2012 13:45:37 -
@@ -403,7 +403,7 @@ dos2unixfn(u_char dn[11], u_char *un, in
* 3 if conversion was successful and generation number was inserted
*/
int
-unix2dosfn(u_char *un, u_char dn[12], int unlen, u_int gen)
+unix2dosfn(u_char *un, u_char dn[11], int unlen, u_int gen)
{
int i, j, l;
int conv = 1;
@@ -416,7 +416,6 @@ unix2dosfn(u_char *un, u_char dn[12], in
*/
for (i = 0; i 11; i++)
dn[i] = ' ';
- dn[11] = 0;
/*
* The filenames . and .. are handled specially, since they
Index: msdosfs_lookup.c
===
RCS file: /cvs/src/sys/msdosfs/msdosfs_lookup.c,v
retrieving revision 1.24
diff -u -p -r1.24 msdosfs_lookup.c
--- msdosfs_lookup.c4 Jul 2011 04:30:41 - 1.24
+++ msdosfs_lookup.c4 Sep 2012 13:38:29 -
@@ -104,7 +104,7 @@ msdosfs_lookup(void *v)
struct msdosfsmount *pmp;
struct buf *bp = 0;
struct direntry *dep;
- u_char dosfilename[12];
+ u_char dosfilename[11];
u_char *adjp;
int adjlen;
int flags;
@@ -193,7 +193,7 @@ msdosfs_lookup(void *v)
slotcount = 0;
#ifdef MSDOSFS_DEBUG
- printf(msdosfs_lookup(): dos version of filename %s, length %d\n,
+ printf(msdosfs_lookup(): dos version of filename '%.11s', length %d\n,
dosfilename, cnp-cn_namelen);
#endif
/*
Re: UQ_BAD_HID
On Thu, Aug 09, 2012 at 03:30:16PM +0100, Stuart Henderson wrote:
Thanks to mpi@, libusb now has some support for communicating
with devices even though they're not attached to ugen(4).
What do people think about removing the UQ_BAD_HID entries in
usb_quirks.c which prevents these devices from attaching to uhid(4)?
My Liebert UPS is okay, yubikeys can be successfully programmed,
and I'll try my wi-spy when I can find it, I wonder if people with
other listed devices could check and see if the listing is still
necessary?
I'm always supportive of removing quirks.
Ken
Index: usb_quirks.c
===
RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
retrieving revision 1.66
diff -u -p -r1.66 usb_quirks.c
--- usb_quirks.c 31 Jan 2012 21:13:32 - 1.66
+++ usb_quirks.c 9 Aug 2012 14:23:41 -
@@ -109,59 +109,6 @@ const struct usbd_quirk_entry {
{ USB_VENDOR_NEC, USB_PRODUCT_NEC_PICTY920, ANY, { UQ_BROKEN_BIDIR }},
{ USB_VENDOR_NEC, USB_PRODUCT_NEC_PICTY800, ANY, { UQ_BROKEN_BIDIR }},
- { USB_VENDOR_APC, USB_PRODUCT_APC_UPS, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APC, USB_PRODUCT_APC_UPS5G,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE_3G,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE_3GS, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE_4_CDMA,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE_4_GSM, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPHONE_4S,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPOD_TOUCH, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPOD_TOUCH_2G,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPOD_TOUCH_3G,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPOD_TOUCH_4G,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPAD, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_IPAD2,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_APPLE, USB_PRODUCT_APPLE_SPEAKERS, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C100, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C120, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C550AVR, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C800, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C900, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C1100,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C1250EITWRK, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6C1500EITWRK, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_BELKIN, USB_PRODUCT_BELKIN_F6H375, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_CYBERPOWER, USB_PRODUCT_CYBERPOWER_1500, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_CYBERPOWER, USB_PRODUCT_CYBERPOWER_OR2200, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_OLD,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_FLASH, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_DELL2, USB_PRODUCT_DELL2_UPS, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_T750, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_T1000, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_T1500, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_RT2200, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_R1500G2,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_HP, USB_PRODUCT_HP_T750G2, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_IDOWELL, USB_PRODUCT_IDOWELL_IDOWELL, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_ITUNER, USB_PRODUCT_ITUNER_USBLCD20x2, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_LIEBERT, USB_PRODUCT_LIEBERT_UPS, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_LIEBERT2, USB_PRODUCT_LIEBERT2_PSA,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_MECANIQUE, USB_PRODUCT_MECANIQUE_WISPY,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_METAGEEK, USB_PRODUCT_METAGEEK_WISPY24I, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_MGE, USB_PRODUCT_MGE_UPS1, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_MGE, USB_PRODUCT_MGE_UPS2, ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_MUSTEK2, USB_PRODUCT_MUSTEK2_PM800,ANY,{ UQ_BAD_HID }},
- { USB_VENDOR_OMRON, USB_PRODUCT_OMRON_BX35F,ANY,{
UQ_BAD_HID
Re: dhclient ignore
I like this on first read. In fact I thought this already existed.
I'll actually look more closely at the code tomorrow.
Ken
On Thu, Jul 26, 2012 at 10:09:28PM -0400, Ted Unangst wrote:
I have a system with two network interfaces (em0 and em1), running dhcp
on both. Both dhcp servers provide me with a nameserver, but only one
of them works (I can't fix this). There is a config file for dhclient
I can use, but it only supports the supersede keyword. I don't want
to statically configure a nameserver override for em1, because the
whole point is that the good nameserver on em0 can change. I just
want to say pretend this option did not arrive.
Diff below adds a little support for an ignore keyword. Like
supersede, except don't actually use the supplied value.
Index: clparse.c
===
RCS file: /cvs/src/sbin/dhclient/clparse.c,v
retrieving revision 1.38
diff -u -p -r1.38 clparse.c
--- clparse.c 10 Dec 2011 17:15:27 - 1.38
+++ clparse.c 27 Jul 2012 01:59:10 -
@@ -170,6 +170,11 @@ parse_client_statement(FILE *cfile)
if (code != -1)
config-default_actions[code] = ACTION_SUPERSEDE;
return;
+ case TOK_IGNORE:
+ code = parse_option_decl(cfile, config-defaults[0]);
+ if (code != -1)
+ config-default_actions[code] = ACTION_IGNORE;
+ return;
case TOK_APPEND:
code = parse_option_decl(cfile, config-defaults[0]);
if (code != -1)
Index: conflex.c
===
RCS file: /cvs/src/sbin/dhclient/conflex.c,v
retrieving revision 1.14
diff -u -p -r1.14 conflex.c
--- conflex.c 10 Dec 2011 17:36:40 - 1.14
+++ conflex.c 27 Jul 2012 01:15:19 -
@@ -337,6 +337,7 @@ static const struct keywords {
{ filename, TOK_FILENAME },
{ fixed-address, TOK_FIXED_ADDR },
{ hardware, TOK_HARDWARE },
+ { ignore, TOK_IGNORE },
{ initial-interval, TOK_INITIAL_INTERVAL },
{ interface, TOK_INTERFACE },
{ lease, TOK_LEASE },
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.146
diff -u -p -r1.146 dhclient.c
--- dhclient.c9 Jul 2012 16:21:21 - 1.146
+++ dhclient.c27 Jul 2012 01:59:35 -
@@ -1535,6 +1535,9 @@ priv_script_write_params(char *prefix, s
if (config-defaults[i].len) {
if (lease-options[i].len) {
switch (config-default_actions[i]) {
+ case ACTION_IGNORE:
+ /* handled below */
+ break;
case ACTION_DEFAULT:
dp = lease-options[i].data;
len = lease-options[i].len;
@@ -1588,6 +1591,9 @@ supersede:
len = lease-options[i].len;
dp = lease-options[i].data;
} else {
+ len = 0;
+ }
+ if (len config-default_actions[i] == ACTION_IGNORE) {
len = 0;
}
if (len) {
Index: dhclient.conf.5
===
RCS file: /cvs/src/sbin/dhclient/dhclient.conf.5,v
retrieving revision 1.21
diff -u -p -r1.21 dhclient.conf.5
--- dhclient.conf.5 9 Apr 2011 19:53:00 - 1.21
+++ dhclient.conf.5 27 Jul 2012 02:05:28 -
@@ -244,6 +244,14 @@ in the
.Ic supersede
statement.
.It Xo
+.Ic ignore No { Op Ar option declaration
+.Oo , Ar ... option declaration Oc }
+.Xc
+If for some set of options the client should always ignore the
+value supplied by the server, these values can be defined in the
+.Ic ignore
+statement.
+.It Xo
.Ic prepend No { Op Ar option declaration
.Oo , Ar ... option declaration Oc }
.Xc
Index: dhcpd.h
===
RCS file: /cvs/src/sbin/dhclient/dhcpd.h,v
retrieving revision 1.76
diff -u -p -r1.76 dhcpd.h
--- dhcpd.h 9 Jul 2012 16:21:21 - 1.76
+++ dhcpd.h 27 Jul 2012 01:18:18 -
@@ -130,6 +130,7 @@ struct client_config {
struct option_data defaults[256];
enum {
ACTION_DEFAULT,
+ ACTION_IGNORE,
ACTION_SUPERSEDE,
ACTION_PREPEND,
ACTION_APPEND
Index: dhctoken.h
===
RCS file: /cvs/src/sbin/dhclient/dhctoken.h,v
retrieving
Re: Virtio drivers for OpenBSD
On Wed, Jul 11, 2012 at 11:02:05AM -0400, Ted Unangst wrote: On Wed, Jul 11, 2012 at 14:07, Stefan Fritsch wrote: Hi, I have been working on porting NetBSD's virtio drivers to OpenBSD. I am not finished yet, but in order to prevent duplicate work, I thought I'd publish the current state (attached as diff to OpenBSD 5.1). It adds a virtio block device driver (viod) and a virtio network interface (if_vioif). It is stable enough to run make -j 2 in /usr/src on a viod disk. Comments are welcome. BTW: Which device numbers should I choose for viod? Use the first unused number or just add virtio at the end? We are consolidating and reducing the number of disk types. I think this would be better if it attached as scsi disks, which is what all the cool virtual disks do these days. And we dream of the day when wd and vnd attach as scsi too! This would be equivalent to visiting Naples for us. 'vedi Napoli e poi muori'. Ken
Re: tedu old comment about cpu affinity.
On Mon, Jul 09, 2012 at 10:47:16AM +0200, Christiano F. Haesbaert wrote: This no longer applies, it is probably a leftover from the days when we had a global runqueue. Discussed with blambert. ok ? Index: sched_bsd.c === RCS file: /cvs/src/sys/kern/sched_bsd.c,v retrieving revision 1.29 diff -d -u -p -r1.29 sched_bsd.c --- sched_bsd.c 23 Mar 2012 15:51:26 - 1.29 +++ sched_bsd.c 9 Jul 2012 08:45:32 - @@ -464,16 +464,7 @@ resched_proc(struct proc *p, u_char pri) /* * XXXSMP - * Since p-p_cpu persists across a context switch, - * this gives us *very weak* processor affinity, in - * that we notify the CPU on which the process last - * ran that it should try to switch. - * - * This does not guarantee that the process will run on - * that processor next, because another processor might - * grab it the next time it performs a context switch. - * - * This also does not handle the case where its last + * This does not handle the case where its last * CPU is running a higher-priority process, but every * other CPU is running a lower-priority process. There * are ways to handle this situation, but they're not If it doesn't apply it should die. ok krw@ Ken
fix dhclient parsing of NVT ASCII options
Apparently we should be removing trailing NULs from options whose
data are NVT ASCII strings.
Other than RFC nitpicking, Microsoft DHCP sometimes sticks those
pesky NULs in, and this breaks 'appending' values via dhclient.conf,
as the pretty print routine will stick '\000' into the generated
string.
Anybody out there suffering from Microsoft DHCP servers that can
verify this helps?
Of course, gentle messages pointing out errors or breakage always
welcome.
Note that DHO_PAD == 0, hence simply shortening the length used to
retrieve the data should leave behind '\0' bytes that will be ignored
as DHO_PAD options.
Ken
Index: options.c
===
RCS file: /cvs/src/sbin/dhclient/options.c,v
retrieving revision 1.39
diff -u -p -r1.39 options.c
--- options.c 11 May 2011 14:38:36 - 1.39
+++ options.c 24 Jun 2012 21:14:27 -
@@ -86,6 +86,19 @@ parse_option_buffer(struct option_data *
warning(rejecting bogus offer.);
return (0);
}
+
+ /*
+* Strip trailing NULs from ascii ('t') options. They
+* will be treated as DHO_PAD options. i.e. ignored. RFC 2132
+* says Options containing NVT ASCII data SHOULD NOT include
+* a trailing NULL; however, the receiver of such options
+* MUST be prepared to delete trailing nulls if they exist.
+*/
+ if (dhcp_options[code].format[0] == 't') {
+ for (len = s[1]; len 0 s[len + 1] == '\0'; len--)
+ ;
+ }
+
/*
* If we haven't seen this option before, just make
* space for it and copy it there.
Re: Memory leak in snmpd(8)
On Thu, May 24, 2012 at 01:54:36PM +0200, Gerhard Roth wrote:
Hi,
with the OPENBSD-CARP-MIB a memory leak was introduced to snmpd(8):
RCS file: mib.c,v
retrieving revision 1.52
diff -u -p -r1.52 mib.c
--- mib.c 2012/03/20 03:01:26 1.52
+++ mib.c 2012/05/24 12:53:35
@@ -2713,7 +2713,7 @@ mib_carpiftable(struct oid *oid, struct ber_oid *o,
st
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
+ if (mib_carpifget(cif, idx) == NULL) {
free(cif);
return (1);
}
Gerhard
--
GeNUA, Gesellschaft fCr Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
Calling mib_carpget() seems a tad over complex. Wouldn't the diff
below make it cleaner? Untested except by gcc.
And doesn't the socket 's' leak too, or does SIOCGVH returning -1
mean 's' was closed?
Ken
Index: mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.52
diff -u -p -r1.52 mib.c
--- mib.c 20 Mar 2012 03:01:26 - 1.52
+++ mib.c 24 May 2012 14:11:22 -
@@ -1360,7 +1360,7 @@ intmib_carpstats(struct oid *, struct
int mib_carpiftable(struct oid *, struct ber_oid *, struct ber_element **);
int mib_carpifnum(struct oid *, struct ber_oid *, struct ber_element **);
struct carpif
- *mib_carpifget(struct carpif *, u_int);
+ *mib_carpifget(u_int);
int mib_memiftable(struct oid *, struct ber_oid *, struct ber_element **);
static struct oid openbsd_mib[] = {
@@ -2647,9 +2647,10 @@ mib_carpifnum(struct oid *oid, struct be
}
struct carpif *
-mib_carpifget(struct carpif *cif, u_int idx)
+mib_carpifget(u_int idx)
{
struct kif *kif;
+ struct carpif *cif;
int s;
struct ifreq ifr;
struct carpreq carpr;
@@ -2692,9 +2693,12 @@ mib_carpifget(struct carpif *cif, u_int
if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
return (NULL);
- memset(cif, 0, sizeof(struct carpif));
- memcpy(cif-carpr, carpr, sizeof(struct carpreq));
- memcpy(cif-kif, kif, sizeof(struct kif));
+ cif = malloc(sizeof(struct carpif));
+ if (cif != NULL) {
+ memset(cif, 0, sizeof(struct carpif));
+ memcpy(cif-carpr, carpr, sizeof(struct carpreq));
+ memcpy(cif-kif, kif, sizeof(struct kif));
+ }
close(s);
@@ -2707,16 +2711,11 @@ mib_carpiftable(struct oid *oid, struct
u_int32_tidx;
struct carpif *cif;
- if ((cif = malloc(sizeof(struct carpif))) == NULL)
- return (1);
-
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
- free(cif);
+ if ((cif = mib_carpifget(idx)) == NULL)
return (1);
- }
/* Tables need to prepend the OID on their own */
o-bo_id[OIDIDX_carpIfEntry] = cif-kif.if_index;
Re: Use a default device for eject(1)
On Sun, May 20, 2012 at 04:46:40PM +0200, Martin Pieuchot wrote: Diff below makes eject(1) use cd0 as default device like cdio(1) does. I'm aware this is an arbitrary choice but I see no drawback in having a default device to eject and this behavior is coherent with cdio(1)'s. Ok? But eject is a tape operation with a default device of st0. This would break stuff for people still using tape devices. Ken
Re: Use a default device for eject(1)
On Sun, May 20, 2012 at 04:46:40PM +0200, Martin Pieuchot wrote: Diff below makes eject(1) use cd0 as default device like cdio(1) does. I'm aware this is an arbitrary choice but I see no drawback in having a default device to eject and this behavior is coherent with cdio(1)'s. Ok? Then again, actual testing shows that I the man page is unclear and a bare 'eject' does not eject st0. I'd rather have eject use 'st0' as the default device since it is a varient of the 'mt' command. This could be made more clear on the man page. :-) Ken
Re: Use a default device for eject(1)
On Sun, May 20, 2012 at 10:36:16AM -0600, Theo de Raadt wrote: On 20/05/12(Sun) 11:26, Kenneth R Westerback wrote: On Sun, May 20, 2012 at 04:46:40PM +0200, Martin Pieuchot wrote: Diff below makes eject(1) use cd0 as default device like cdio(1) does. I'm aware this is an arbitrary choice but I see no drawback in having a default device to eject and this behavior is coherent with cdio(1)'s. Ok? Then again, actual testing shows that I the man page is unclear and a bare 'eject' does not eject st0. I'd rather have eject use 'st0' as the default device since it is a varient of the 'mt' command. Forget about the default, as I said it was an arbitrary choice and there is no consensus. This could be made more clear on the man page. :-) Actually I see no reason at all to mention that eject(1) is a variant of mt(1) because it is not limited to tapes and the manual is confusing. So I've split the manual in two, does it look clearer to you? I don't see the point of splitting the manual page. What problem are you trying to solve? The idea is that eject is mt offl, and has all the same behaviours, and everything else in the manual page is interesting to read for both of them. I agree. I don't see any point in splitting them. The source code and executables are identical with very small difference in option handling, so it makes more sense to me to keep them together. Ken
Re: Do you want to do any manual network configuration?
On Thu, Apr 19, 2012 at 08:01:19PM +0200, Henning Brauer wrote: * Theo de Raadt dera...@cvs.openbsd.org [2012-04-19 18:44]: I needed it in different situations. install over wifi with a wep/wpa key, old crappy hw that need explicit media settings and maybe other cases that I forgot. For sure I can interrupt the installation, setup the net manually and restart it, but I don't really see a benefit in removing that question. ^Z or ! works anywhere. exactly. and the installer points out the ! way right at the beginning. I intend to commit this with the 3 oks I got, if people strongly disagree speak up quickly. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/ ok krw@ if not too late. I do wonder if there is a place for a general 'Manual configuration' prompt/pause. Which would be also a nice place to put the opt-contemplated Accept all defaults from now on option. Ken
Re: diff: improving msdosfs write speed for large files
On Wed, Apr 04, 2012 at 03:51:38PM +0200, Mike Belopuhov wrote:
On Wed, Apr 04, 2012 at 14:42 +0400, Alexander Polakov wrote:
This is a diff from NetBSD pr.34583:
http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=34583
Quoting the author:
I noticed that when writing large file (hundreds of megabytes)
to an msdos disk, the writing speed to a file decreases with the
file length.
Since I have some experience with messydos filesystems (I wrote
MSH: for the Amiga) I took a look.
The obvious suspicion with operations that slow down with the
length of a file is an excessive traversal of the FAT cluster
chain. However, there is a cache that caches 2 positions: the
last cluster in the file, and the most recently looked up one.
Debugging info showed however that frequent full traversals were
still made. So, apparently when extending a file and after
updating the end cluster, the previous end is again needed.
Adding a 3rd entry in the cache, which keeps the end position
from just before extending a file.
This has the desired effect of keeping the write speed constant.
(What it is that needs that position I have not been able to
ascertain from the filesystem code; it doesn't seem to make
sense, actually, to read or write clusters before the original
EOF. I was hoping to find the place where the cache is trashed
and rewrite it to get the desired info from it beforehand, so
that the extra cache entry is again unneeded, but alas.)
While there, I changed 0 to NULL for two pointer arguments of
extendfile().
i agree that this is a great find. i don't really like the diff though.
i see no point in introducing this macro. what do others think?
Agreed. I like this version better.
Ken
Index: msdosfs/denode.h
===
RCS file: /cvs/src/sys/msdosfs/denode.h,v
retrieving revision 1.23
diff -u -p -r1.23 denode.h
--- msdosfs/denode.h 17 Jul 2010 19:27:07 - 1.23
+++ msdosfs/denode.h 4 Apr 2012 12:20:23 -
@@ -116,10 +116,11 @@ struct fatcache {
* cache is probably pretty worthless if a file is opened by multiple
* processes.
*/
-#define FC_SIZE 2 /* number of entries in the cache */
+#define FC_SIZE 3 /* number of entries in the cache */
#define FC_LASTMAP 0 /* entry the last call to pcbmap()
resolved
* to */
#define FC_LASTFC 1 /* entry for the last cluster in the
file */
+#define FC_OLASTFC 2 /* entry for the previous last cluster
*/
#define FCE_EMPTY 0x /* doesn't represent an actual
cluster # */
Index: msdosfs/msdosfs_fat.c
===
RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v
retrieving revision 1.22
diff -u -p -r1.22 msdosfs_fat.c
--- msdosfs/msdosfs_fat.c 4 Jul 2011 04:30:41 - 1.22
+++ msdosfs/msdosfs_fat.c 4 Apr 2012 12:20:26 -
@@ -952,6 +952,13 @@ extendfile(struct denode *dep, uint32_t
return (error);
}
+ /*
+ * Preserve value for the last cluster before extending the file
+ * to speed up further lookups.
+ */
+ fc_setcache(dep, FC_OLASTFC, dep-de_fc[FC_LASTFC].fc_frcn,
+ dep-de_fc[FC_LASTFC].fc_fsrcn);
+
while (count 0) {
/*
* Allocate a new cluster chain and cat onto the end of the
Re: Sun Fire v215 panic at boot
On Tue, Mar 13, 2012 at 08:15:33PM +0100, Mark Kettenis wrote: Date: Mon, 12 Mar 2012 22:39:31 +0100 (CET) From: Mark Kettenis mark.kette...@xs4all.nl Date: Sat, 25 Feb 2012 09:55:57 +0100 From: Paul de Weerd we...@weirdnet.nl I recently got a v215 from a friend and have installed OpenBSD on it. Occassionally, it will panic during boot. This happened during install and I see it now during regular reboots. I can pretty much reproduce this at will with a couple of reboots. Could this be faulty hardware ? To reset the ALOM password, I installed Solaris 10 (took an eternity) and that never showed any problems, but I guess that doesn't prove much. First the panic and then full dmesg (from a succesful boot) are included below. I doubt this is faulty hardware. I've seen similar reports for a v445, which has the same crappy Acer Labs pciide(4) controller. I fear that the wdc.c changes made in April 2011 introduced this behaviour. So thanks to Paul giving me access to the machine in question I've been able to figure out what's going wrong here. The data error always happens when running wdcintr() for channel 1. Now on these machines we have the following line in dmesg ... pciide0: channel 1 disabled (no drives) ... indicating that there is no actual hardware connected to channel 1. As a result of this we skip further initialization of the channel. Therefore it shouldn't be a terrible surprise that the chip doesn't like it when we try to read registers associated with this channel. On crappy PC hardware this won't be noticed, but on sparc64 this results in an unrecoverable fault. The solution is easy. We shouldn't be calling wdcintr() for a channel that isn't properly initialized. ok? Index: pciide.c === RCS file: /cvs/src/sys/dev/pci/pciide.c,v retrieving revision 1.337 diff -u -p -r1.337 pciide.c --- pciide.c 15 Jan 2012 15:16:23 - 1.337 +++ pciide.c 13 Mar 2012 18:54:50 - @@ -1838,6 +1838,9 @@ pciide_pci_intr(void *arg) if (cp-compat) continue; + if (cp-hw_ok == 0) + continue; + if (pciide_intr_flag(cp) == 0) continue; Make sense to me. ok krw@
Re: installation to (W)hole disk - saner default
On Wed, Mar 07, 2012 at 05:32:09PM +0100, David Vasek wrote: Hello all. While I would always defend everybody's right to use OpenBSD to shoot himself in his foot, I don't think it is neither practical nor ethical to hint him to do so. So if the installer finds a valid MBR which contains some partition(s), then don't make whole disk (overwriting everything) the default choice and let it up to the user. For those who still want to use whole disk in this not so frequent case, it requires exactly one key press more. Will it fit on the floppies and is it is worth the extra 16 bytes? (Yes, it can be made in a little more compact way.) Regards, David Corrupt MBR's and the prevalence of installing one OS per disk led to the current choice. It is STRONGLY preferred to use the whole disk for the boot disk, so why should we encourage anything else? If you want to avoid accidentally zapping your disk just fdisk it to have the desired OpenBSD partition BEFORE running the OpenBSD install. Ken
Tweak MBR/FAT spoofing
1) There is no place on a FAT drive to put a disklabel. So when asked where
to write a disklabel on a FAT device, return an error. I chose ENXIO, but
can easily be argued around to something else.
2) When deciding if we have processed one or more MBR/EBR's check the
number of MBR/EBR's we have transited and not the number of partitions
we have spoofed as a result. Increment the count after deciding if we
processing MBR's.
3) If there is a missing signature on the first block, and we thus
decide it isn't an MBR, don't skip checking if it's a FAT device. The
signature should also be there for FAT, but we have other tests to
ensure we are looking at a FAT, and some devices do not have the
signature. And Windows doesn't seem to care.
4) Rename some labels to make more sense. Various other minor cleanups
and comment editing.
This fixes getting a spoofed 'i' partition on my HiMD media, FAT
formatted with 2048-byte sectors.
Thoughts? People with odd spoofing requirements sought as testers. :-).
Ken
Index: subr_disk.c
===
RCS file: /cvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.143
diff -u -p -r1.143 subr_disk.c
--- subr_disk.c 10 Feb 2012 18:41:36 - 1.143
+++ subr_disk.c 24 Feb 2012 01:27:40 -
@@ -406,7 +406,6 @@ readdoslabel(struct buf *bp, void (*stra
* Map the partitions to disklabel entries i-p
*/
while (wander loop DOS_MAXEBR) {
- loop++;
wander = 0;
if (part_blkno extoff)
part_blkno = extoff;
@@ -428,16 +427,19 @@ readdoslabel(struct buf *bp, void (*stra
bcopy(bp-b_data + offset, dp, sizeof(dp));
- if (n == 0 part_blkno == DOSBBSECTOR) {
- u_int16_t fattest;
+ if (loop == 0 part_blkno == DOSBBSECTOR) {
+ u_int16_t mbrtest;
/* Check the end of sector marker. */
- fattest = ((bp-b_data[510] 8) 0xff00) |
+ mbrtest = ((bp-b_data[510] 8) 0xff00) |
(bp-b_data[511] 0xff);
- if (fattest != 0x55aa)
- goto notfat;
+ if (mbrtest != 0x55aa)
+ break;
}
+ /* Keep track of # of mbr's traversed. */
+ loop++;
+
if (ourpart == -1) {
/* Search for our MBR partition */
for (dp2=dp, i=0; i NDOSPART ourpart == -1;
@@ -446,7 +448,7 @@ readdoslabel(struct buf *bp, void (*stra
dp2-dp_typ == DOSPTYP_OPENBSD)
ourpart = i;
if (ourpart == -1)
- goto donot;
+ goto spoofmbrparts;
/*
* This is our MBR partition. need sector
* address for SCSI/IDE, cylinder for
@@ -458,7 +460,7 @@ readdoslabel(struct buf *bp, void (*stra
/* found our OpenBSD partition, finish up */
if (partoffp)
- goto notfat;
+ goto done;
if (lp-d_ntracks == 0)
lp-d_ntracks = dp2-dp_ehd + 1;
@@ -468,11 +470,7 @@ readdoslabel(struct buf *bp, void (*stra
lp-d_secpercyl = lp-d_ntracks *
lp-d_nsectors;
}
-donot:
- /*
-* In case the disklabel read below fails, we want to
-* provide a fake label in i-p.
-*/
+spoofmbrparts:
for (dp2=dp, i=0; i NDOSPART; i++, dp2++) {
struct partition *pp;
u_int8_t fstype;
@@ -515,7 +513,6 @@ donot:
part_blkno = 0;
}
wander = 1;
- continue;
break;
default:
fstype = FS_OTHER;
@@ -523,6 +520,9 @@ donot:
}
/*
+* If we are 'wandering', i.e. partition was an
+* EXTEND/EXTENDL, there is no spoofing to be done.
+*
* Don't set fstype/offset/size when just looking for
* the offset of the OpenBSD partition. It would
* invalidate the disklabel checksum!
@@ -530,7 +530,7 @@ donot:
* Don't try to spoof more than 8 partitions, i.e.
* 'i' -'p'.
*/
- if
Re: readdir man page
On Thu, Feb 02, 2012 at 10:29:00PM -0700, Philip Guenther wrote:
On Thu, 2 Feb 2012, Philip Guenther wrote:
I also think readdir() should set errno if it detects an invalid
seekdir(). EINVAL seems correct.
Here's a diff for this bit.
oks?
Philip Guenther
Index: gen/readdir.c
===
RCS file: /cvs/src/lib/libc/gen/readdir.c,v
retrieving revision 1.15
diff -u -p -r1.15 readdir.c
--- gen/readdir.c 18 Nov 2009 07:43:22 - 1.15
+++ gen/readdir.c 3 Feb 2012 05:21:58 -
@@ -29,6 +29,7 @@
*/
#include dirent.h
+#include errno.h
#include thread_private.h
/*
@@ -52,12 +53,14 @@ _readdir_unlocked(DIR *dirp, struct dire
return (-1);
}
dp = (struct dirent *)(dirp-dd_buf + dirp-dd_loc);
- if ((long)dp 03) /* bogus pointer check */
- return (-1);
- if (dp-d_reclen = 0 ||
- dp-d_reclen dirp-dd_len + 1 - dirp-dd_loc)
+ if ((long)dp 03 ||/* bogus pointer check */
+ dp-d_reclen = 0 ||
+ dp-d_reclen dirp-dd_len + 1 - dirp-dd_loc) {
+ errno = EINVAL;
return (-1);
+ }
dirp-dd_loc += dp-d_reclen;
+
/*
* When called from seekdir(), we let it decide on
* the end condition to avoid overshooting: the next
I like this diff per se, just not enough of a POSIX lawyer to say if the
subtly different behaviour will set off alarms in Austin or wherever POSIX
hangs out these days. :-)
Ken
Re: transferred/transferring typos
On Sat, Jan 07, 2012 at 02:40:24PM +, Jason McIntyre wrote:
On Sat, Jan 07, 2012 at 03:29:40PM +0100, Tobias Ulmer wrote:
After typing 'transferring' wrong one time too many...
I didn't touch gcc, binutils, bind, lynx, kerberos, openssl or perl on
purpose.
ok by me.
jmc
Me too. ok krw@. Typos are very distracting when reading code.
Ken
Index: lib/libsndio/sio_sun.c
===
RCS file: /home/vcs/cvs/openbsd/src/lib/libsndio/sio_sun.c,v
retrieving revision 1.4
diff -u -p -r1.4 sio_sun.c
--- lib/libsndio/sio_sun.c 15 Nov 2011 08:05:22 - 1.4
+++ lib/libsndio/sio_sun.c 7 Jan 2012 14:11:33 -
@@ -48,7 +48,7 @@ struct sio_sun_hdl {
int fd;
int filling;
unsigned ibpf, obpf;/* bytes per frame */
- unsigned ibytes, obytes;/* bytes the hw transfered */
+ unsigned ibytes, obytes;/* bytes the hw transferred */
unsigned ierr, oerr;/* frames the hw dropped */
int offset; /* frames play is ahead of record */
int idelta, odelta; /* position reported to client */
Index: sys/arch/macppc/dev/tpms.c
===
RCS file: /home/vcs/cvs/openbsd/src/sys/arch/macppc/dev/tpms.c,v
retrieving revision 1.15
diff -u -p -r1.15 tpms.c
--- sys/arch/macppc/dev/tpms.c 9 Apr 2010 17:01:30 - 1.15
+++ sys/arch/macppc/dev/tpms.c 7 Jan 2012 14:11:45 -
@@ -128,7 +128,7 @@
* Magic numbers.
*/
-/* The amount of data transfered by the USB device. */
+/* The amount of data transferred by the USB device. */
#define TPMS_DATA_LEN 81
/* The maximum number of sensors. */
Index: sys/arch/vax/dec/sii.c
===
RCS file: /home/vcs/cvs/openbsd/src/sys/arch/vax/dec/sii.c,v
retrieving revision 1.13
diff -u -p -r1.13 sii.c
--- sys/arch/vax/dec/sii.c 17 Jul 2011 22:46:47 - 1.13
+++ sys/arch/vax/dec/sii.c 7 Jan 2012 14:11:51 -
@@ -645,7 +645,7 @@ again:
printf(%s: Parity error\n, sc-sc_dev.dv_xname);
goto abort;
}
- /* dmalen = amount left to transfer, i = amount transfered */
+ /* dmalen = amount left to transfer, i = amount transferred */
i = state-dmalen;
state-dmalen = 0;
state-dmaCurPhase = -1;
@@ -899,7 +899,7 @@ again:
#endif
}
- /* read amount transfered if DMA didn't finish */
+ /* read amount transferred if DMA didn't finish */
if (state-dmalen 0) {
i = state-dmalen - regs-dmlotc;
state-dmalen = 0;
Index: sys/dev/ata/wdvar.h
===
RCS file: /home/vcs/cvs/openbsd/src/sys/dev/ata/wdvar.h,v
retrieving revision 1.18
diff -u -p -r1.18 wdvar.h
--- sys/dev/ata/wdvar.h 22 Sep 2011 22:12:45 - 1.18
+++ sys/dev/ata/wdvar.h 7 Jan 2012 14:11:57 -
@@ -44,8 +44,8 @@ struct ata_bio {
struct disklabel *lp; /* pointer to drive's label info */
daddr64_t blkno; /* block addr */
daddr64_t blkdone; /* number of blks transferred */
-daddr64_t nblks; /* number of block currently transfering */
-int nbytes; /* number of bytes currently transfering */
+daddr64_t nblks; /* number of block currently transferring */
+int nbytes; /* number of bytes currently transferring */
longbcount; /* total number of bytes */
char *databuf; /* data buffer address */
volatile int error;
Index: sys/dev/ic/aic79xx.c
===
RCS file: /home/vcs/cvs/openbsd/src/sys/dev/ic/aic79xx.c,v
retrieving revision 1.48
diff -u -p -r1.48 aic79xx.c
--- sys/dev/ic/aic79xx.c19 Apr 2011 21:59:51 - 1.48
+++ sys/dev/ic/aic79xx.c7 Jan 2012 14:11:57 -
@@ -1225,7 +1225,7 @@ ahd_handle_seqint(struct ahd_softc *ahd,
* that requires host assistance for completion.
* While handling the message phase(s), we will be
* notified by the sequencer after each byte is
-* transfered so we can track bus phase changes.
+* transferred so we can track bus phase changes.
*
* If this is the first time we've seen a HOST_MSG_LOOP
* interrupt, initialize the state of the host message
Index: sys/dev/ic/aic79xx.h
===
RCS file: /home/vcs/cvs/openbsd/src/sys/dev/ic/aic79xx.h,v
retrieving revision 1.21
diff -u -p -r1.21 aic79xx.h
--- sys/dev/ic/aic79xx.h21 Dec 2006
Re: ntfs: respect the MNT_FORCE flag upon unmount
On Tue, Dec 20, 2011 at 01:33:37AM +0100, Mike Belopuhov wrote:
this is an improved diff that addresses the problem with forced
unmount of the ntfs filesystem in situations like the one described
here: http://marc.info/?l=openbsd-bugsm=132257956328474w=2
ntfs keeps a bunch of vnodes opened and marked as VSYSTEM including
the mount point: it's usecount is 1 and it is incremented if you do
cd /mnt. when you pull out a usb stick and ntfs_unmount is called
it flushes all but system vnodes and then checks if system vnodes'
usecount is not more than 1 (somebody is using a vnode). in case
this is not true (and it's not true for the mount point since shell
process is holding that vnode) it returns EBUSY.
the only thing is ntfs is a read-only filesystem so there's no
real reason to not to succeed and forceclose all the vnodes.
the following change makes it respect the MNT_FORCE flag and proceed
even if there's someone holding a vnode. also it silences the
ntfs_reclaim (like ufs_reclaim is silenced by the prtactive).
it might not be a perfect diff, but it improves the situation by
a vast margin and still reports fs as being busy when unmount is
not forced (e.g. you run umount(1)).
comments/ok?
I don't use NTFS, but this makes sense to me. ok krw@.
Ken
Index: ntfs/ntfs_vfsops.c
===
RCS file: /cvs/src/sys/ntfs/ntfs_vfsops.c,v
retrieving revision 1.27
diff -u -p -r1.27 ntfs_vfsops.c
--- ntfs/ntfs_vfsops.c4 Jul 2011 20:35:35 - 1.27
+++ ntfs/ntfs_vfsops.c20 Dec 2011 00:12:06 -
@@ -547,10 +547,12 @@ ntfs_unmount(
return (error);
}
- /* Check if only system vnodes are rest */
- for(i=0;iNTFS_SYSNODESNUM;i++)
- if((ntmp-ntm_sysvn[i])
- (ntmp-ntm_sysvn[i]-v_usecount 1)) return (EBUSY);
+ /* Check if system vnodes are still referenced */
+ for(i=0;iNTFS_SYSNODESNUM;i++) {
+ if(((mntflags MNT_FORCE) == 0) (ntmp-ntm_sysvn[i]
+ ntmp-ntm_sysvn[i]-v_usecount 1))
+ return (EBUSY);
+ }
/* Dereference all system vnodes */
for(i=0;iNTFS_SYSNODESNUM;i++)
Index: ntfs/ntfs_vnops.c
===
RCS file: /cvs/src/sys/ntfs/ntfs_vnops.c,v
retrieving revision 1.24
diff -u -p -r1.24 ntfs_vnops.c
--- ntfs/ntfs_vnops.c 4 Jul 2011 20:35:35 - 1.24
+++ ntfs/ntfs_vnops.c 20 Dec 2011 00:12:06 -
@@ -72,7 +72,7 @@ static int ntfs_bmap(void *);
static int ntfs_fsync(void *);
static int ntfs_pathconf(void *);
-int ntfs_prtactive = 1; /* 1 = print out reclaim of active vnodes */
+int ntfs_prtactive = 0; /* 1 = print out reclaim of active vnodes */
/*
* This is a noop, simply returning what one has been given.
Re: rc.d/xdm, check for broken xkb
On Tue, Nov 01, 2011 at 04:39:29PM +, Stuart Henderson wrote:
If you hit the xkb file/directory problem (for example, if you follow
the upgrading without install kernel instructions), you can't type at
the keyboard, even to switch to a text console.
What does anyone think about disabling xdm if this brokenness is found?
Index: xdm
===
RCS file: /cvs/src/etc/rc.d/xdm,v
retrieving revision 1.1
diff -u -p -r1.1 xdm
--- xdm 7 Jul 2011 18:42:17 - 1.1
+++ xdm 1 Nov 2011 16:35:39 -
@@ -6,4 +6,10 @@ daemon=/usr/X11R6/bin/xdm
. /etc/rc.d/rc.subr
+rc_pre() {
+ # XXX Mitigate xkb mistake in 5.0, better not to run xdm than
+ # leave a broken keyboard
+ [ -d /usr/X11R6/share/X11/xkb/symbols/srvr_ctrl ] return 1
+}
+
rc_cmd $1
I haven't encountered the problem, but if xdm just failed I would be
somewhat surprised. Is there a useful message logged somewhere that would
indicate why xdm isn't starting? If there were such a message I think
this is a good idea.
Ken
Re: rc.d/xdm, check for broken xkb
On Wed, Nov 02, 2011 at 08:42:15AM -0400, Brad wrote:
On 02/11/11 8:31 AM, Kenneth R Westerback wrote:
On Tue, Nov 01, 2011 at 04:39:29PM +, Stuart Henderson wrote:
If you hit the xkb file/directory problem (for example, if you follow
the upgrading without install kernel instructions), you can't type at
the keyboard, even to switch to a text console.
What does anyone think about disabling xdm if this brokenness is found?
Index: xdm
===
RCS file: /cvs/src/etc/rc.d/xdm,v
retrieving revision 1.1
diff -u -p -r1.1 xdm
--- xdm 7 Jul 2011 18:42:17 - 1.1
+++ xdm 1 Nov 2011 16:35:39 -
@@ -6,4 +6,10 @@ daemon=/usr/X11R6/bin/xdm
. /etc/rc.d/rc.subr
+rc_pre() {
+ # XXX Mitigate xkb mistake in 5.0, better not to run xdm than
+ # leave a broken keyboard
+ [ -d /usr/X11R6/share/X11/xkb/symbols/srvr_ctrl ] return 1
+}
+
rc_cmd $1
I haven't encountered the problem, but if xdm just failed I would be
somewhat surprised. Is there a useful message logged somewhere that would
indicate why xdm isn't starting? If there were such a message I think
this is a good idea.
Ken
Try rereading what Stuart said.
OK, I've re-read it. Still says the same thing. Keyboard non-functional.
I just tried to point out that the suggested solution might be
improved by ensuring an explanatory message is emitted or logged
somewhere so it would be obvious to me why xdm didn't start. And asked
if such a message were already emitted somewhere.
Ken
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: rc.d/xdm, check for broken xkb
On Wed, Nov 02, 2011 at 02:00:47PM +0100, Stefan Sperling wrote:
On Wed, Nov 02, 2011 at 08:31:19AM -0400, Kenneth R Westerback wrote:
On Tue, Nov 01, 2011 at 04:39:29PM +, Stuart Henderson wrote:
If you hit the xkb file/directory problem (for example, if you follow
the upgrading without install kernel instructions), you can't type at
the keyboard, even to switch to a text console.
What does anyone think about disabling xdm if this brokenness is found?
Index: xdm
===
RCS file: /cvs/src/etc/rc.d/xdm,v
retrieving revision 1.1
diff -u -p -r1.1 xdm
--- xdm 7 Jul 2011 18:42:17 - 1.1
+++ xdm 1 Nov 2011 16:35:39 -
@@ -6,4 +6,10 @@ daemon=/usr/X11R6/bin/xdm
. /etc/rc.d/rc.subr
+rc_pre() {
+ # XXX Mitigate xkb mistake in 5.0, better not to run xdm than
+ # leave a broken keyboard
+ [ -d /usr/X11R6/share/X11/xkb/symbols/srvr_ctrl ] return 1
+}
+
rc_cmd $1
I haven't encountered the problem, but if xdm just failed I would be
somewhat surprised. Is there a useful message logged somewhere that would
indicate why xdm isn't starting? If there were such a message I think
this is a good idea.
The script might as well try to fix the issue.
Index: xdm
===
RCS file: /cvs/src/etc/rc.d/xdm,v
retrieving revision 1.1
diff -u -p -r1.1 xdm
--- xdm 7 Jul 2011 18:42:17 - 1.1
+++ xdm 2 Nov 2011 12:54:26 -
@@ -6,4 +6,14 @@ daemon=/usr/X11R6/bin/xdm
. /etc/rc.d/rc.subr
+pre_rc() {
+ # XXX Try to fix xkb mistake in 5.0
+ _symbols_dir=/usr/X11R6/share/X11/xkb/symbols
+ if [ -f ${_symbols_dir}/srvr_ctrl/srvr_ctrl ]; then
+ mv ${_symbols_dir}/srvr_ctrl ${_symbols_dir}/_srvr_ctrl
+ mv ${_symbols_dir}/_srvr_ctrl/srvr_ctrl ${_symbols_dir}
+ rmdir ${_symbols_dir}/_srvr_ctrl
+ fi
+}
+
rc_cmd $1
Even better.
Ken
Re: use strtonum to check Port statements in apache
On Thu, Aug 25, 2011 at 08:25:00PM +1000, David Gwynne wrote:
i want to push my set scheme in http diff again, but it makes
sense to reuse server_port as part of that.
this makes server_port more palatable to me.
ok?
Can't see that this changes anything, and it does look better, so
ok krw@.
Ken
Index: src/main/http_core.c
===
RCS file: /cvs/src/usr.sbin/httpd/src/main/http_core.c,v
retrieving revision 1.27
diff -u -p -r1.27 http_core.c
--- src/main/http_core.c 10 May 2010 02:00:50 - 1.27
+++ src/main/http_core.c 25 Aug 2011 10:23:02 -
@@ -1779,14 +1779,16 @@ static const char *server_port(cmd_parms
if (err != NULL) {
return err;
}
-port = atoi(arg);
-if (port = 0 || port = 65536) { /* 65536 == 116 */
- return ap_pstrcat(cmd-temp_pool, The port number \, arg,
- \ is outside the appropriate range
- (i.e., 1..65535)., NULL);
+
+port = (int)strtonum(arg, 1, 65535, err);
+if (err != NULL) {
+ return ap_pstrcat(cmd-temp_pool,
+ The port number \, arg, \ is , err, NULL);
}
+
cmd-server-port = port;
-return NULL;
+
+return (NULL);
}
static const char *set_signature_flag(cmd_parms *cmd, core_dir_config *d,
Re: mg dired diff to reduce annoyance
On Sat, Jul 30, 2011 at 04:30:42PM -0400, Loganaden Velvindron wrote: I'll wait for kjell@ to have some time to review this diff. No big deal really :-D You might wait a long time. Kjell has transitioned to the real world and now works for a living. :-) Ken
Re: s/firmwares/firmware/, it's already plural
On Wed, Jul 27, 2011 at 10:49:01PM +0200, David Coppa wrote:
Didn't know about this...
http://wiki.answers.com/Q/What_is_the_plural_of_firmware
ok for me
Me too.
Ken
On Wed, Jul 27, 2011 at 10:31 PM, Stuart Henderson s...@spacehopper.org
wrote:
ok?
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.647
diff -u -p -r1.647 install.sub
--- distrib/miniroot/install.sub14 Jul 2011 14:54:57 -
1.647
+++ distrib/miniroot/install.sub27 Jul 2011 20:29:51 -
@@ -1643,12 +1643,12 @@ run_sysmerge() {
fi
}
-update_firmwares() {
+update_firmware() {
local _get=Install
[[ $MODE == upgrade ]] _get=Update
- ask_yn $_get non-free firmwares on first boot? no
+ ask_yn $_get non-free firmware on first boot? no
[[ $resp == y ]] \
- echo echo 'updating firmwares'; /usr/sbin/fw_update -v \
+ echo echo 'updating firmware'; /usr/sbin/fw_update -v \
/mnt/etc/rc.firsttime
}
@@ -1991,7 +1991,7 @@ finish_up() {
[[ $MODE == upgrade ]] run_sysmerge
- update_firmwares
+ update_firmware
# Pat on the back.
cat __EOT
Index: usr.sbin/fw_update/fw_update.sh
===
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.7
diff -u -p -r1.7 fw_update.sh
--- usr.sbin/fw_update/fw_update.sh 27 Jul 2011 15:12:57 - 1.7
+++ usr.sbin/fw_update/fw_update.sh 27 Jul 2011 20:29:51 -
@@ -66,21 +66,21 @@ for driver in $DRIVERS; do
done
if [ -z $install$update ]; then
- verbose No devices found which need firmwares to be downloaded.
+ verbose No devices found which need firmware to be downloaded.
exit 0
fi
[ $nop ] || [ 0 = $(id -u) ] ||
{ echo ${0##*/} must be run as root 2; exit 1; }
-# Install missing firmwares
+# Install missing firmware
if [ $install ]; then
- verbose Installing firmwares:$install.
+ verbose Installing firmware:$install.
$PKG_ADD $nop $verbose $install
fi
-# Update installed firmwares
+# Update installed firmware
if [ $update ]; then
- verbose Updating firmwares:$update.
+ verbose Updating firmware:$update.
$PKG_ADD $nop $verbose -u $update
fi
Re: impossible panic() in ifafree()
On Fri, Jul 22, 2011 at 02:51:32PM +0200, Martin Pelikan wrote:
Hi,
this panic cannot possibly happen. The question is, whether to move it up
to the macro that is used in the code, or zap it entirely.
The whole logic in these two seems pretty ugly, but inet6 crashed when I
tried to make it look prettier and more uniform. And again, until 5.0 is
out, this might just sit here, or wait on someone's todo list.
--
Martin Pelikan
Index: net/if.h
===
RCS file: /cvs/src/sys/net/if.h,v
retrieving revision 1.128
diff -u -p -r1.128 if.h
--- net/if.h 8 Jul 2011 18:48:51 - 1.128
+++ net/if.h 22 Jul 2011 12:14:01 -
@@ -696,6 +696,8 @@ __END_DECLS
#ifdef _KERNEL
#define IFAFREE(ifa) \
do { \
+ if (ifa == NULL) \
+ panic(IFAFREE); \
if ((ifa)-ifa_refcnt = 0) \
ifafree(ifa); \
else \
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.131
diff -u -p -r1.131 route.c
--- net/route.c 4 Jul 2011 04:29:17 - 1.131
+++ net/route.c 22 Jul 2011 12:14:01 -
@@ -411,8 +411,6 @@ rtfree(struct rtentry *rt)
void
ifafree(struct ifaddr *ifa)
{
- if (ifa == NULL)
- panic(ifafree);
if (ifa-ifa_refcnt == 0)
free(ifa, M_IFADDR);
else
What is the point of the #define? If anything I'd go the other way
and move the refcount check into ifafree(). Saving a function call
at the expense of bloating the kernel has, in other cases, been
shown to be a losing proposition. I can only see fewer than 30
IFAFREE() occurances in /usr/src/*.c.
Ken
Relax 2^28-1 (128G) installboot restriction
Having a hard limit to ensure that OpenBSD will be able to boot on
i386 and amd64 has smoked out at least some people who were
successfully booting past that limit. So back off the error to a
warning for now.
ok? (Only compile tested on amd64 so far)
Ken
Index: amd64/stand/installboot/installboot.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/installboot/installboot.c,v
retrieving revision 1.22
diff -u -p -r1.22 installboot.c
--- amd64/stand/installboot/installboot.c 5 Jul 2011 18:34:10 -
1.22
+++ amd64/stand/installboot/installboot.c 18 Jul 2011 22:35:26 -
@@ -243,12 +243,12 @@ write_bootblocks(int devfd, struct diskl
errx(1, no OpenBSD partition);
}
- if (start + (protosize / dl-d_secsize) BOOTBIOS_MAXSEC)
- errx(1, invalid location: all of /boot must be sector %u.,
- BOOTBIOS_MAXSEC);
-
if (verbose)
fprintf(stderr, /boot will be written at sector %u\n, start);
+
+ if (start + (protosize / dl-d_secsize) BOOTBIOS_MAXSEC)
+ warnx(/boot extends beyond sector %u. OpenBSD may not boot.,
+ BOOTBIOS_MAXSEC);
if (!nowrite) {
if (lseek(devfd, (off_t)start * dl-d_secsize, SEEK_SET) 0)
Index: amd64/stand/libsa/biosdev.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.15
diff -u -p -r1.15 biosdev.c
--- amd64/stand/libsa/biosdev.c 17 Mar 2011 12:53:44 - 1.15
+++ amd64/stand/libsa/biosdev.c 18 Jul 2011 22:35:26 -
@@ -216,10 +216,6 @@ EDD_rw(int rw, int dev, u_int32_t daddr,
int rv;
volatile static struct EDD_CB cb;
- /* Some (most?) BIOSen get confused by i/o above 2 ^ 28 - 1 sector. */
- if ((daddr + nblk) BOOTBIOS_MAXSEC)
- return (1); /* Invalid function/parameter. */
-
/* Zero out reserved stuff */
cb.edd_res1 = 0;
cb.edd_res2 = 0;
Index: i386/stand/installboot/installboot.c
===
RCS file: /cvs/src/sys/arch/i386/stand/installboot/installboot.c,v
retrieving revision 1.65
diff -u -p -r1.65 installboot.c
--- i386/stand/installboot/installboot.c5 Jul 2011 18:34:10 -
1.65
+++ i386/stand/installboot/installboot.c18 Jul 2011 22:35:26 -
@@ -239,12 +239,12 @@ write_bootblocks(int devfd, struct diskl
errx(1, no OpenBSD partition);
}
- if (start + (protosize / dl-d_secsize) BOOTBIOS_MAXSEC)
- errx(1, invalid location: all of /boot must be sector %u.,
- BOOTBIOS_MAXSEC);
-
if (verbose)
fprintf(stderr, /boot will be written at sector %u\n, start);
+
+ if (start + (protosize / dl-d_secsize) BOOTBIOS_MAXSEC)
+ warnx(/boot extends beyond sector %u. OpenBSD may not boot.,
+ BOOTBIOS_MAXSEC);
if (!nowrite) {
if (lseek(devfd, (off_t)start * dl-d_secsize, SEEK_SET) 0)
Index: i386/stand/libsa/biosdev.c
===
RCS file: /cvs/src/sys/arch/i386/stand/libsa/biosdev.c,v
retrieving revision 1.83
diff -u -p -r1.83 biosdev.c
--- i386/stand/libsa/biosdev.c 17 Mar 2011 12:53:44 - 1.83
+++ i386/stand/libsa/biosdev.c 18 Jul 2011 22:35:26 -
@@ -217,10 +217,6 @@ EDD_rw(int rw, int dev, u_int32_t daddr,
int rv;
volatile static struct EDD_CB cb;
- /* Some (most?) BIOSen get confused by i/o above 2 ^ 28 - 1 sector. */
- if ((daddr + nblk) BOOTBIOS_MAXSEC)
- return (1); /* Invalid function/parameter. */
-
/* Zero out reserved stuff */
cb.edd_res1 = 0;
cb.edd_res2 = 0;
Re: malloc flags: even more strict 'S'
On Tue, Jul 12, 2011 at 01:23:52PM +0200, Otto Moerbeek wrote:
Hi,
at the cost of some speed, reduce the malloc cache size to 0 with
flag 'S'. This means that pages that become free will be unmapped asap.
This detects more use-after-free bugs. The slowdown is because of more
unmap/mmap calls.
ok?
-Otto
I like this too. ok krw@ no matter how slow it is. :-).
Ken
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.138
diff -u -p -r1.138 malloc.c
--- malloc.c 20 Jun 2011 18:04:06 - 1.138
+++ malloc.c 12 Jul 2011 11:18:41 -
@@ -68,6 +68,8 @@
#define MALLOC_MAXCACHE 256
#define MALLOC_DELAYED_CHUNKS15 /* max of getrnibble() */
#define MALLOC_INITIAL_REGIONS 512
+#define MALLOC_DEFAULT_CACHE 64
+
/*
* When the P option is active, we move allocations between half a page
* and a whole page towards the end, subject to alignment constraints.
@@ -461,7 +463,7 @@ omalloc_init(struct dir_info **dp)
*/
mopts.malloc_abort = 1;
mopts.malloc_move = 1;
- mopts.malloc_cache = 64;
+ mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
for (i = 0; i 3; i++) {
switch (i) {
@@ -551,10 +553,12 @@ omalloc_init(struct dir_info **dp)
case 's':
mopts.malloc_freeprot = mopts.malloc_junk = 0;
mopts.malloc_guard = 0;
+ mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
break;
case 'S':
mopts.malloc_freeprot = mopts.malloc_junk = 1;
mopts.malloc_guard = MALLOC_PAGESIZE;
+ mopts.malloc_cache = 0;
break;
case 'x':
mopts.malloc_xmalloc = 0;
Re: small fix in ehci
On Sat, Jul 09, 2011 at 06:44:26AM +0200, Eric Faurot wrote: So, there is actually another bug in that chunk of code. This diff fixes them: - Read the register from the correct location: HCSPARAMS is a capability register. - Construct the resulting mask correctly by adding parenthesis where needed: '|' takes precedence over '?' so currently the expression evaluates as EHCI_HCS_PPC(v) ? UHD_PWR_INDIVIDUAL : UHD_PORT_IND where v is not even the correct value. This should let the ehci driver correctly report the characteristics of the controller's root hub. ok krw@ Ken Index: ehci.c === RCS file: /cvs/src/sys/dev/usb/ehci.c,v retrieving revision 1.117 diff -u -r1.117 ehci.c --- ehci.c3 Jul 2011 15:47:17 - 1.117 +++ ehci.c9 Jul 2011 00:52:49 - @@ -2148,11 +2148,10 @@ } hubd = ehci_hubd; hubd.bNbrPorts = sc-sc_noport; - v = EOREAD4(sc, EHCI_HCSPARAMS); + v = EREAD4(sc, EHCI_HCSPARAMS); USETW(hubd.wHubCharacteristics, - EHCI_HCS_PPC(v) ? UHD_PWR_INDIVIDUAL : UHD_PWR_NO_SWITCH | - EHCI_HCS_P_INDICATOR(EREAD4(sc, EHCI_HCSPARAMS)) - ? UHD_PORT_IND : 0); + (EHCI_HCS_PPC(v) ? UHD_PWR_INDIVIDUAL : UHD_PWR_NO_SWITCH) | + (EHCI_HCS_P_INDICATOR(v) ? UHD_PORT_IND : 0)); hubd.bPwrOn2PwrGood = 200; /* XXX can't find out? */ for (i = 0, l = sc-sc_noport; l 0; i++, l -= 8, v = 8) hubd.DeviceRemovable[i++] = 0; /* XXX can't find out? */
More disklabel fixes (hppa/hppa64/macppc/sgi)
If we have successfully found and read in the disk block we expect
to hold the OpenBSD disklabel, then what is there must be treated
as the disklabel. If it is currently invalid (e.g. all zeros) we
still want to write the new label in this MD location, and thus
should pass the spoofed label back up the stack. We DON'T want
to put the label in the readdoslabel() location.
This fixes the easy arches with readMDlabel() functions so they
can (again?) be formatted using the native tools and OpenBSD can
put its disklabel in the expected place.
This also fixes the boundstart/boundend values returned so the
OpenBSD 'area' is initialized to the specificied area of disk.
Alpha/mac68k/sparc/sparc64 fixes will follow.
ok?
Ken
Index: hppa/hppa/disksubr.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/disksubr.c,v
retrieving revision 1.82
diff -u -p -r1.82 disksubr.c
--- hppa/hppa/disksubr.c8 Jul 2011 00:08:00 - 1.82
+++ hppa/hppa/disksubr.c8 Jul 2011 20:22:12 -
@@ -234,8 +234,15 @@ finished:
goto done;
}
- error = checkdisklabel(bp-b_data + LABELOFFSET, lp, openbsdstart,
- DL_GETDSIZE(lp)); /* XXX */
+ /*
+* Do OpenBSD disklabel validation/adjustment.
+*
+* N.B: No matter what the bits are on the disk, we now have the
+* OpenBSD disklabel for this lif disk. DO NOT proceed to
+* readdoslabel(), iso_spooflabel(), etc.
+*/
+ checkdisklabel(bp-b_data, lp, openbsdstart, DL_GETDSIZE(lp));
+ error = 0;
done:
if (dbp) {
Index: hppa64/hppa64/disksubr.c
===
RCS file: /cvs/src/sys/arch/hppa64/hppa64/disksubr.c,v
retrieving revision 1.66
diff -u -p -r1.66 disksubr.c
--- hppa64/hppa64/disksubr.c8 Jul 2011 00:08:00 - 1.66
+++ hppa64/hppa64/disksubr.c8 Jul 2011 20:46:56 -
@@ -234,8 +234,15 @@ finished:
goto done;
}
- error = checkdisklabel(bp-b_data + LABELOFFSET, lp, openbsdstart,
- DL_GETDSIZE(lp)); /* XXX */
+ /*
+* Do OpenBSD disklabel validation/adjustment.
+*
+* N.B: No matter what the bits are on the disk, we now have the
+* OpenBSD disklabel for this lif disk. DO NOT proceed to
+* readdoslabel(), iso_spooflabel(), etc.
+*/
+ checkdisklabel(bp-b_data, lp, openbsdstart, DL_GETDSIZE(lp));
+ error = 0;
done:
if (dbp) {
Index: macppc/macppc/disksubr.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/disksubr.c,v
retrieving revision 1.74
diff -u -p -r1.74 disksubr.c
--- macppc/macppc/disksubr.c8 Jul 2011 00:08:00 - 1.74
+++ macppc/macppc/disksubr.c8 Jul 2011 20:50:19 -
@@ -183,8 +183,16 @@ readdpmelabel(struct buf *bp, void (*str
if (biowait(bp))
return(bp-b_error);
- return checkdisklabel(bp-b_data + LABELOFFSET, lp, hfspartoff,
- hfspartend);
+ /*
+* Do OpenBSD disklabel validation/adjustment.
+*
+* N.B: No matter what the bits are on the disk, we now have the
+* disklabel for this dpme disk. DO NOT proceed to readdoslabel(),
+* iso_spooflabel(), * etc.
+*/
+ checkdisklabel(bp-b_data + LABELOFFSET, lp, openbsdstart,
+ DL_GETDSIZE(lp));
+ return (0);
}
/*
Index: sgi/sgi/disksubr.c
===
RCS file: /cvs/src/sys/arch/sgi/sgi/disksubr.c,v
retrieving revision 1.24
diff -u -p -r1.24 disksubr.c
--- sgi/sgi/disksubr.c 8 Jul 2011 00:08:00 - 1.24
+++ sgi/sgi/disksubr.c 8 Jul 2011 20:50:49 -
@@ -206,7 +206,16 @@ finished:
if (biowait(bp))
return (bp-b_error);
- return checkdisklabel(bp-b_data + offset, lp, fsoffs, fsend);
+ /*
+* Do OpenBSD disklabel validation/adjustment.
+*
+* N.B: No matter what the bits are on the disk, we now have the
+* OpenBSD disklabel for this sgi disk. DO NOT proceed to
+* readdoslabel(), iso_spooflabel(), etc.
+*/
+ checkdisklabel(bp-b_data + LABELOFFSET, lp, openbsdstart,
+ DL_GETDSIZE(lp));
+ return (0);
}
/*
Re: Kludge to 'fix' sdmmc_scsi.c
On Fri, Jul 08, 2011 at 04:34:36PM -0700, Matthew Dempsky wrote:
autoconf doesn't allow you to attach multiple child devices to the
same device with different attach arg types. sdmmc(4) tries to attach
both scsibus(4) and SDIO devices (currently just the disabled sbt(4)
bluetooth adapter). The way it does this is by creating a
sdmmc_attach_args structure that was compatible with
scsibus_attach_args, and then new SDIO-specific fields at the bottom.
However, I've added new fields to scsibus_attach_args, which
sdmmc_attach_args doesn't know about. The stupid kludge for now is to
just include a complete struct scsibus_attach_args in
sdmmc_attach_args instead of replicating its fields.
There are better fixes for this, but the sdmmc stuff is too tangled,
and I want to finish with my SCSI diffs before messing with this more.
ok?
sure. ok krw@
Ken
Index: sdmmcvar.h
===
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/sdmmc/sdmmcvar.h,v
retrieving revision 1.18
diff -u -p -r1.18 sdmmcvar.h
--- sdmmcvar.h24 Aug 2010 14:52:23 - 1.18
+++ sdmmcvar.h8 Jul 2011 23:27:33 -
@@ -182,7 +182,7 @@ struct sdmmc_softc {
* Attach devices at the sdmmc bus.
*/
struct sdmmc_attach_args {
- struct scsi_link *scsi_link;/* XXX */
+ struct scsibus_attach_args saa; /* XXX */
struct sdmmc_function *sf;
};
Index: sdmmc_scsi.c
===
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/dev/sdmmc/sdmmc_scsi.c,v
retrieving revision 1.27
diff -u -p -r1.27 sdmmc_scsi.c
--- sdmmc_scsi.c 16 Jun 2011 01:09:16 - 1.27
+++ sdmmc_scsi.c 8 Jul 2011 23:28:14 -
@@ -139,9 +139,9 @@ sdmmc_scsi_attach(struct sdmmc_softc *sc
scbus-sc_link.pool = scbus-sc_iopool;
bzero(saa, sizeof(saa));
- saa.scsi_link = scbus-sc_link;
+ saa.saa.saa_sc_link = scbus-sc_link;
- scbus-sc_child = config_found(sc-sc_dev, saa, scsiprint);
+ scbus-sc_child = config_found(sc-sc_dev, saa.saa, scsiprint);
if (scbus-sc_child == NULL) {
printf(%s: can't attach scsibus\n, sc-sc_dev.dv_xname);
goto free_ccbs;
More archs should write disklabel where it will be read
Without changing behaviour for native disklabels, allow some more archs that
call
readdoslabel() to read/write disklabels to same place as readdoslabel() reads
from.
The hppa paren chunk are to make hppa and hppa64 disksubr.c identical
again.
ok?
Ken
Index: hppa/hppa/disksubr.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/disksubr.c,v
retrieving revision 1.80
diff -u -p -r1.80 disksubr.c
--- hppa/hppa/disksubr.c16 Apr 2011 03:21:15 - 1.80
+++ hppa/hppa/disksubr.c7 Jul 2011 04:55:45 -
@@ -108,7 +108,7 @@ readliflabel(struct buf *bp, void (*stra
SET(bp-b_flags, B_BUSY | B_READ | B_RAW);
(*strat)(bp);
if (biowait(bp))
- return bp-b_error;
+ return (bp-b_error);
lvp = (struct lifvol *)bp-b_data;
if (lvp-vol_id != LIF_VOL_ID) {
@@ -252,6 +252,7 @@ int
writedisklabel(dev_t dev, void (*strat)(struct buf *), struct disklabel *lp)
{
int error = EIO, partoff = -1;
+ int offset;
struct disklabel *dlp;
struct buf *bp = NULL;
@@ -259,12 +260,17 @@ writedisklabel(dev_t dev, void (*strat)(
bp = geteblk((int)lp-d_secsize);
bp-b_dev = dev;
- if (readliflabel(bp, strat, lp, partoff, 1) != 0
- readdoslabel(bp, strat, lp, partoff, 1) != 0)
+ if (readliflabel(bp, strat, lp, partoff, 1) == 0) {
+ bp-b_blkno = partoff + LABELSECTOR;
+ offset = LABELOFFSET;
+ } else if (readdoslabel(bp, strat, lp, partoff, 1) == 0) {
+ bp-b_blkno = DL_BLKTOSEC(lp, partoff + LABELSECTOR) *
+ DL_BLKSPERSEC(lp);
+ offset = DL_BLKOFFSET(lp, partoff + LABELSECTOR) + LABELOFFSET;
+ } else
goto done;
/* Read it in, slap the new label in, and write it back out */
- bp-b_blkno = partoff + LABELSECTOR;
bp-b_bcount = lp-d_secsize;
CLR(bp-b_flags, B_READ | B_WRITE | B_DONE);
SET(bp-b_flags, B_BUSY | B_READ | B_RAW);
@@ -272,7 +278,7 @@ writedisklabel(dev_t dev, void (*strat)(
if ((error = biowait(bp)) != 0)
goto done;
- dlp = (struct disklabel *)(bp-b_data + LABELOFFSET);
+ dlp = (struct disklabel *)(bp-b_data + offset);
*dlp = *lp;
CLR(bp-b_flags, B_READ | B_WRITE | B_DONE);
SET(bp-b_flags, B_BUSY | B_WRITE | B_RAW);
Index: hppa64/hppa64/disksubr.c
===
RCS file: /cvs/src/sys/arch/hppa64/hppa64/disksubr.c,v
retrieving revision 1.64
diff -u -p -r1.64 disksubr.c
--- hppa64/hppa64/disksubr.c16 Apr 2011 03:21:15 - 1.64
+++ hppa64/hppa64/disksubr.c7 Jul 2011 04:57:30 -
@@ -252,6 +252,7 @@ int
writedisklabel(dev_t dev, void (*strat)(struct buf *), struct disklabel *lp)
{
int error = EIO, partoff = -1;
+ int offset;
struct disklabel *dlp;
struct buf *bp = NULL;
@@ -259,8 +260,14 @@ writedisklabel(dev_t dev, void (*strat)(
bp = geteblk((int)lp-d_secsize);
bp-b_dev = dev;
- if (readliflabel(bp, strat, lp, partoff, 1) != 0
- readdoslabel(bp, strat, lp, partoff, 1) != 0)
+ if (readliflabel(bp, strat, lp, partoff, 1) == 0) {
+ bp-b_blkno = partoff + LABELSECTOR;
+ offset = LABELOFFSET;
+ } else if (readdoslabel(bp, strat, lp, partoff, 1) == 0) {
+ bp-b_blkno = DL_BLKTOSEC(lp, partoff + LABELSECTOR) *
+ DL_BLKSPERSEC(lp);
+ offset = DL_BLKOFFSET(lp, partoff + LABELSECTOR) + LABELOFFSET;
+ } else
goto done;
/* Read it in, slap the new label in, and write it back out */
@@ -272,7 +279,7 @@ writedisklabel(dev_t dev, void (*strat)(
if ((error = biowait(bp)) != 0)
goto done;
- dlp = (struct disklabel *)(bp-b_data + LABELOFFSET);
+ dlp = (struct disklabel *)(bp-b_data + offset);
*dlp = *lp;
CLR(bp-b_flags, B_READ | B_WRITE | B_DONE);
SET(bp-b_flags, B_BUSY | B_WRITE | B_RAW);
Index: macppc/macppc/disksubr.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/disksubr.c,v
retrieving revision 1.72
diff -u -p -r1.72 disksubr.c
--- macppc/macppc/disksubr.c16 Apr 2011 03:21:15 - 1.72
+++ macppc/macppc/disksubr.c7 Jul 2011 15:19:29 -
@@ -194,6 +194,7 @@ int
writedisklabel(dev_t dev, void (*strat)(struct buf *), struct disklabel *lp)
{
int error = EIO, partoff = -1;
+ int offset;
struct disklabel *dlp;
struct buf *bp = NULL;
@@ -201,12 +202,17 @@ writedisklabel(dev_t dev, void (*strat)(
bp = geteblk((int)lp-d_secsize);
bp-b_dev = dev;
- if (readdpmelabel(bp, strat, lp, partoff, 1) != 0
- readdoslabel(bp, strat, lp, partoff, 1) != 0)
+ if (readdpmelabel(bp, strat,
Re: memsets in bind
On Tue, Jul 05, 2011 at 06:54:49PM +, Miod Vallat wrote:
found by jsg.
Index: lib/isc/hmacsha.c
===
RCS file: /home/tedu/cvs/src/usr.sbin/bind/lib/isc/hmacsha.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 hmacsha.c
--- lib/isc/hmacsha.c 9 Dec 2007 12:34:04 - 1.1.1.1
+++ lib/isc/hmacsha.c 5 Jul 2011 18:43:15 -
@@ -65,7 +65,7 @@ void
isc_hmacsha1_invalidate(isc_hmacsha1_t *ctx) {
isc_sha1_invalidate(ctx-sha1ctx);
memset(ctx-key, 0, sizeof(ctx-key));
- memset(ctx, 0, sizeof(ctx));
+ memset(ctx, 0, sizeof(*ctx));
Then what purpose is there to keep ctx-key memset before?
Ditto. Otherwise ok krw@.
Ken
Re: fdisk geom fixes
On Mon, Jul 04, 2011 at 02:26:42PM -0700, andre...@zoho.com wrote:
cmd.c:
x86* maxhead should be 255, not 256
make geom less useless by using disklabel for cyl, if available
geom is useless, period. Making it easier will only encourage people
to use it.
part.c:
x86* maxhead should be 254, not 255 (other arches?)
But 6 lines ago you say maxhead should be 255 not 256? Perhaps you can
provide the valid ranges for all of these and your references for
the claim. An example on how you manage to cause yourself a problem
would also be helpful.
maxcyl gets set to 1023 before this function gets called, so it's a
redundant check
Index: src/sbin/fdisk/cmd.c
===
RCS file: /cvs/src/sbin/fdisk/cmd.c,v
retrieving revision 1.45
diff -p -u -r1.45 cmd.c
--- src/sbin/fdisk/cmd.c 2 Jul 2010 02:54:09 - 1.45
+++ src/sbin/fdisk/cmd.c 4 Jul 2011 20:55:22 -
@@ -67,9 +67,10 @@ Xreinit(cmd_t *cmd, disk_t *disk, mbr_t
int
Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *mbr, mbr_t *tt, int offset)
{
- int maxcyl = 1024;
- int maxhead = 256;
- int maxsec = 63;
+ u_int32_t maxcyl = 1024;
+ u_int32_t maxhead = 255;
+ u_int32_t maxsec = 63;
+ u_int32_t dmaxcyl = 0;
What's the point of u_int32_t?
/* Print out disk info */
DISK_printmetrics(disk, cmd-args);
@@ -80,8 +81,15 @@ Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *m
maxsec = 999;
#endif
+ if (disk-label)
+ dmaxcyl = disk-label-cylinders;
+
/* Ask for new info */
if (ask_yn(Change disk geometry?)) {
+ if (dmaxcyl maxcyl) {
+ printf(Warning setting cyl beyond %u forces LBA\n,
maxcyl);
+ maxcyl = dmaxcyl;
+ }
Since ask_num won't permit numbers max, how will LBA be forced?
disk-real-cylinders = ask_num(BIOS Cylinders, ASK_DEC,
disk-real-cylinders, 1, maxcyl, NULL);
disk-real-heads = ask_num(BIOS Heads, ASK_DEC,
Index: src/sbin/fdisk/part.c
===
RCS file: /cvs/src/sbin/fdisk/part.c,v
retrieving revision 1.50
diff -p -u -r1.50 part.c
--- src/sbin/fdisk/part.c 29 Apr 2009 22:58:24 - 1.50
+++ src/sbin/fdisk/part.c 4 Jul 2011 20:38:24 -
@@ -212,12 +212,10 @@ PRT_parse(disk_t *disk, void *prt, off_t
int
PRT_check_chs(prt_t *partn)
{
- if ( (partn-shead 255) ||
+ if ( (partn-shead 254) ||
(partn-ssect 63) ||
- (partn-scyl 1023) ||
- (partn-ehead 255) ||
- (partn-esect 63) ||
- (partn-ecyl 1023) )
+ (partn-ehead 254) ||
+ (partn-esect 63) )
{
return 0;
}
Ken
Re: Refactor disk driver code
On Mon, Jun 27, 2011 at 03:22:13PM -0400, Kenneth R Westerback wrote: On Mon, Jun 27, 2011 at 12:01:51PM -0700, Matthew Dempsky wrote: The diff below adds some very common disk driver logic into subr_disk.c and refactors most of the MI disk drivers to take advantage of them. I'll followup with the MD disk drivers later (a lot of them need other cleanups anyway). There should be no behavioral change. The only part of the diff that might not be intuitive is in cd.c and sd.c, the 'out' label is effectively moved to before the partition validity check, but that's okay because 'goto out' is only run when opening the raw partition (which is still unconditionally allowed by disk_openpart()). ok? Reads nice on first pass. I'll toss it on my boxes and see what falls off. Just not sure if I have wd anywhere anymore ... Ken sd(4)/cd(4) working fine via ahci on my amd64 box. wd(4) working fine on my macppc. ok krw@ Ken
