Re: Brainy: a few bugs

[Daniel Dickman]

On Fri, Sep 11, 2015 at 3:18 PM, Maxime Villard  wrote:
>
> _19/ UNINITIALIZED VARIABLE: sys/arch/i386/i386/bios.c rev1.112
>

Fixed. Thanks.

Re: panic with latest ugen.c and pcsc-lite

[David Coppa]

On Tue, Sep 15, 2015 at 6:50 AM, Grant Czajkowski  wrote:
> On Fri, Sep 11, 2015 at 02:41:04AM -0600, David Coppa wrote:
>>
>> Hi!
>>
>> Repeatedly hit the panic below with latest ugen.c code (v 1.88) and
>> pcsc-lite.
>
> Thanks for the report.  Could you please try this patch?

Yes, this fixes the panic.

Ciao!
David

Re: [patch] if-free cleanup in sys/arch

[Miod Vallat]

> Hi,
> 
> Here a first sets of "if(x) free(x)" cleanup in sys/arch/
> 
> This patch contains only trivial if(x) removal. The size argument in
> free is keep untouched (because it is already setted, or because it
> makes sens to keep it to 0).
> 
> Comments ? OK ?

The arch/*/stand use a free() implementation which does not accept NULL
pointers (see sys/lib/libsa/alloc.c).

All the other instances are ok.

Re: panic with latest ugen.c and pcsc-lite

[Martin Pieuchot]

On 15/09/15(Tue) 04:50, Grant Czajkowski wrote:
> On Fri, Sep 11, 2015 at 02:41:04AM -0600, David Coppa wrote:
> > 
> > Hi!
> > 
> > Repeatedly hit the panic below with latest ugen.c code (v 1.88) and
> > pcsc-lite.
> 
> Thanks for the report.  Could you please try this patch?

ok mpi@

Grant what do you you think about doing an audit of the tree to see if
we're missing this check in other drivers?  I might be interesting to
search bugs@ archives for similar reports.

> Index: ugen.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.88
> diff -u -p -d -r1.88 ugen.c
> --- ugen.c7 Sep 2015 19:58:42 -   1.88
> +++ ugen.c15 Sep 2015 04:42:02 -
> @@ -557,7 +557,9 @@ ugen_do_read(struct ugen_softc *sc, int 
>   flags, sce->timeout, NULL);
>   err = usbd_transfer(xfer);
>   if (err) {
> - usbd_clear_endpoint_stall(sce->pipeh);
> + if (err == USBD_STALLED)
> + usbd_clear_endpoint_stall(sce->pipeh);
> +
>   if (err == USBD_INTERRUPTED)
>   error = EINTR;
>   else if (err == USBD_TIMEOUT)
> @@ -691,7 +693,9 @@ ugen_do_write(struct ugen_softc *sc, int
>   flags, sce->timeout, NULL);
>   err = usbd_transfer(xfer);
>   if (err) {
> - usbd_clear_endpoint_stall(sce->pipeh);
> + if (err == USBD_STALLED)
> + usbd_clear_endpoint_stall(sce->pipeh);
> +
>   if (err == USBD_INTERRUPTED)
>   error = EINTR;
>   else if (err == USBD_TIMEOUT)
> 

Re: [patch] if-free cleanup in sys/arch

[Rafael Neves]

On Tue, Sep 15, 2015 at 8:37 PM, Ted Unangst  wrote:

> Miod Vallat wrote:
> > > Hi,
> > >
> > > Here a first sets of "if(x) free(x)" cleanup in sys/arch/
> > >
> > > This patch contains only trivial if(x) removal. The size argument in
> > > free is keep untouched (because it is already setted, or because it
> > > makes sens to keep it to 0).
> > >
> > > Comments ? OK ?
> >
> > The arch/*/stand use a free() implementation which does not accept NULL
> > pointers (see sys/lib/libsa/alloc.c).
>
> But we're going to fix that too, right?
>
>
It seems that now NULL can be passed to this free implmenation. This has
changed yesterday in revision 1.11 (
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/lib/libsa/alloc.c.diff?r1=1.10=1.11=h
).

Regards,
Rafael Neves

Re: Call for testers of restricted rmt(8)

[Alexander Hall]

On 09/12/15 09:13, Sebastien Marie wrote:
> First, some generals remarks:
>
> - The debug feature (not documented) defeat the `-r' flag purpose.

How do you mean it defeats it? The purpose of the '-r' flag is to stop
a user from creating and/or writing to files. Obviously said user may
not dictate the rmt arguments himself in that case.

>
>I think the code should be either:
>  - enclosed in #ifdef DEBUG (prefered way)
>  - not permitted if `rflag' or `wflag' are setted

I was tempted to rip that undocumented feature out entirely. But that's
another diff, so I left it as-is. The only regression the diff
introduces is if you are currently using a debug log filename with a
leading dash. I won't care about that.

> - Adding tame(2) may be a good way to enforce the policyi, but it should
>be added later (when other userland tools gains it).

Sure, later.

> Else, just some comments inline.
>
> On Thu, Sep 10, 2015 at 12:58:52AM +0200, Alexander Hall wrote:
>>
>> Index: rmt.c
>> ===
>> RCS file: /cvs/src/usr.sbin/rmt/rmt.c,v
>> retrieving revision 1.15
>> diff -u -p -r1.15 rmt.c
>> --- rmt.c16 Jan 2015 06:40:20 -  1.15
>> +++ rmt.c9 Sep 2015 22:57:41 -

>> @@ -93,10 +132,66 @@ top:
>>  getstring(device, sizeof(device));
>>  getstring(mode, sizeof(mode));
>>  DEBUG2("rmtd: O %s %s\n", device, mode);
>> -tape = open(device, atoi(mode),
>> -S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
>> +
>> +devp = device;
>> +f = atoi(mode);
>
> atoi(3) is a bit fragile.

Yeah, but the code is full of it already, so I left it consistent and
potentially for another diff to fix.

>> +m = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH;
>> +acc = f & O_ACCMODE;
>> +if (dir) {
>> +/* Strip away valid directory prefix. */
>> +if (strncmp(dir, devp, dirlen) == 0 &&
>> +(devp[dirlen - 1] == '/' ||
>> + devp[dirlen] == '/')) {
>> + devp += dirlen;
>> + while (*devp == '/')
>> +devp++;
>> +}
>
> the "strip away valid directory prefix" code is a bit complex. If I
> understand your purpose, you deal with possible trailing '/' in `dir' in
> the same code than removing the prefix from `devp'.

Yes, and also potential extra slashes in the attempted filename
("rmt -d /some/path" vs the filename "/some/path//filename")

While maybe somewhat complex, do you think it's wrong?

> maybe you should:
>
>1. canonalize `dir' (by removing possible trailing '/') (should be
>done in if (dir) { } block before).
>
>>> if (dir[dirlen] == '/') {
>>> dir[dirlen] = '\0';
>>> dirlen --;
>>> }

>2. remove `dir' prefix from `devp'
>
>>> if (strncmp(dir, devp, dirlen) == 0 && devp[dirlen] == '/')
>>>devp += dirlen + 1;

FWIW, this does not handle multiple slashes in the attempted filename.


>> +} else if (wflag) {
>> +/*
>> + * Require, and force creation of, a nonexistant file,
>> + * unless we are reopening the last opened file again,
>> + * in which case it is opened read-only.
>> + */
>> +if (strcmp(devp, lastdevice) != 0) {
>> +/*
>> + * Disallow read-only open since that would
>> + * only result in an empty file.
>> + */
>> +if (acc == O_RDONLY) {
>> +errno = EPERM;
>> +goto ioerror;
>> +}
>
> does when using -w, O_RDWR should be permitted ? I would expect
> O_WRONLY to be the only allowed mode.
>
>> +f |= O_CREAT | O_EXCL;
>
> anyway, O_CREAT | O_EXCL would mitigate the use of O_RDWR for reading
> existing file...

Yeah, I considered that being fine. -r and -w is not strictly read-only
and write-only in every low-level sense, but at a slightly higher level.

>> +} else {
>> +acc = O_RDONLY;
>> +}
>> +/* Create readonly file */
>> +m = S_IRUSR|S_IRGRP|S_IROTH;
>> +}
>> +/* Apply new access mode. */
>> +f = (f & ~O_ACCMODE) | acc;
>> +
>> +tape = open(device, f, m);
>
> we have checked if `devp' is safe, so we should use `devp' instead of
> `device'.

I was thinking it didn't matter much, but yes, we're less likely to be
bitten by race conditions that way.

That said, this is the only thing I feel compelled to change. If you
feel strongly otherwise, please convince me.

New diff 

Re: [patch] if-free cleanup in sys/arch

[Michael McConville]

Miod Vallat wrote:
> > Hi,
> > 
> > Here a first sets of "if(x) free(x)" cleanup in sys/arch/
> > 
> > This patch contains only trivial if(x) removal. The size argument in
> > free is keep untouched (because it is already setted, or because it
> > makes sens to keep it to 0).
> > 
> > Comments ? OK ?
> 
> The arch/*/stand use a free() implementation which does not accept NULL
> pointers (see sys/lib/libsa/alloc.c).

Is there a good reason for this? POSIX requires that free() be
NULL-safe. This may not apply to the kernel's free(), but it seems like
something that programmers would expect.

Re: [patch] if-free cleanup in sys/arch

[Miod Vallat]

> > The arch/*/stand use a free() implementation which does not accept NULL
> > pointers (see sys/lib/libsa/alloc.c).
> 
> But we're going to fix that too, right?

Yes. Developers those days are expecting free(NULL) to work, and as long
as it's easy to achieve, there is no reason to deprive them from that
sugar.

Re: Brainy: a few bugs

[Mark Kettenis]

> From: Maxime Villard 
> Date: Fri, 11 Sep 2015 21:18:18 +0200
>
> _22/ OVERLAP: sys/arch/sparc64/dev/vdsp.c rev1.2

Thanks, fixed!

Re: Brainy: a few bugs

[Alexander Bluhm]

On Mon, Sep 14, 2015 at 06:53:06AM +0200, Claudio Jeker wrote:
> Fix for _17

OK bluhm@

> 
> -- 
> :wq Claudio
> 
> Index: netinet/if_ether.c
> ===
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 if_ether.c
> --- netinet/if_ether.c13 Sep 2015 17:53:44 -  1.168
> +++ netinet/if_ether.c14 Sep 2015 04:42:08 -
> @@ -367,10 +367,11 @@ arpresolve(struct ifnet *ifp, struct rte
>   "local address\n", __func__, inet_ntop(AF_INET,
>   (dst)->sin_addr, addr, sizeof(addr)));
>   } else {
> + la = NULL;
>   if ((rt = arplookup(satosin(dst)->sin_addr.s_addr, 1, 0,
>   ifp->if_rdomain)) != NULL)
>   la = ((struct llinfo_arp *)rt->rt_llinfo);
> - else
> + if (la == NULL)
>   log(LOG_DEBUG, "%s: %s: can't allocate llinfo\n",
>   __func__,
>   inet_ntop(AF_INET, (dst)->sin_addr,

Cache-Control for httpd

[Manuel Giraud]

Hi,

Here is a patch for some cache control in httpd. I've embedded this into
the 'types' section so one can have the following in httpd.conf:

types {
text/html   htm html
text/csscss < 604800
image/jpeg  jpeg jpg < 86400
}

Questions:
- Is there any interest?
- Is the right place (maybe this shoud be in 'location')?

Index: http.h
===
RCS file: /cvs/src/usr.sbin/httpd/http.h,v
retrieving revision 1.12
diff -u -p -r1.12 http.h
--- http.h  11 Feb 2015 12:52:01 -  1.12
+++ http.h  15 Sep 2015 12:18:20 -
@@ -208,21 +208,22 @@ struct http_mediatype {
char*media_name;
char*media_type;
char*media_subtype;
+   int media_maxage;
 };
 /*
  * Some default media types based on (2014-08-04 version):
  * https://www.iana.org/assignments/media-types/media-types.xhtml
  */
 #define MEDIA_TYPES{   \
-   { "css","text", "css" },\
-   { "html",   "text", "html" },   \
-   { "txt","text", "plain" },  \
-   { "gif","image","gif" },\
-   { "jpeg",   "image","jpeg" },   \
-   { "jpg","image","jpeg" },   \
-   { "png","image","png" },\
-   { "svg","image","svg+xml" },\
-   { "js", "application",  "javascript" }, \
+   { "css","text", "css",  0 },\
+   { "html",   "text", "html", 0 },\
+   { "txt","text", "plain",0 },\
+   { "gif","image","gif",  0 },\
+   { "jpeg",   "image","jpeg", 0 },\
+   { "jpg","image","jpeg", 0 },\
+   { "png","image","png",  0 },\
+   { "svg","image","svg+xml",  0 },\
+   { "js", "application",  "javascript",   0 },\
{ NULL }\
 }
 
Index: httpd.h
===
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.81
diff -u -p -r1.81 httpd.h
--- httpd.h 23 Feb 2015 18:43:18 -  1.81
+++ httpd.h 15 Sep 2015 12:18:20 -
@@ -457,6 +457,7 @@ struct media_type {
char media_type[MEDIATYPE_TYPEMAX];
char media_subtype[MEDIATYPE_TYPEMAX];
char*media_encoding;
+   int  media_maxage;
RB_ENTRY(media_type) media_entry;
 };
 RB_HEAD(mediatypes, media_type);
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.65
diff -u -p -r1.65 parse.y
--- parse.y 12 Feb 2015 04:40:23 -  1.65
+++ parse.y 15 Sep 2015 12:18:20 -
@@ -960,7 +960,15 @@ mediaoptsl : STRING '/' STRING {
}
free($1);
free($3);
-   } medianames_l optsemicolon
+   } medianames_l optmaxage optsemicolon {
+   if (!loadcfg)
+   break;
+
+   if (media_add(conf->sc_mediatypes, ) == NULL) {
+   yyerror("failed to add media type");
+   YYERROR;
+   }
+   }
| include
;
 
@@ -977,15 +985,18 @@ medianamesl   : numberstring  
{
YYERROR;
}
free($1);
+   }
+   ;
 
-   if (!loadcfg)
-   break;
-
-   if (media_add(conf->sc_mediatypes, ) == NULL) {
-   yyerror("failed to add media type");
+optmaxage  : '<' NUMBER {
+   if ($2 < 0) {
+   yyerror("invalid maxage: %lld", $2);
YYERROR;
}
+
+   media.media_maxage = $2;
}
+   |
;
 
 port   : PORT NUMBER {
@@ -1553,6 +1564,7 @@ load_config(const char *filename, struct
(void)strlcpy(m.media_subtype,
mediatypes[i].media_subtype,
sizeof(m.media_subtype));
+   m.media_maxage = mediatypes[i].media_maxage;
m.media_encoding = NULL;
 
if (media_add(conf->sc_mediatypes, ) == NULL) {
Index: server_http.c

ping6 cleanup

[David Hill]

Hello -

Remove unused defines.  Change pr_addr to take socklen_t for
getnameinfo.

Index: ping6.c
===
RCS file: /cvs/src/sbin/ping6/ping6.c,v
retrieving revision 1.115
diff -u -p -r1.115 ping6.c
--- ping6.c 12 Sep 2015 11:52:23 -  1.115
+++ ping6.c 15 Sep 2015 13:22:11 -
@@ -131,7 +131,6 @@ struct payload {
 #defineEXTRA   256 /* for AH and various other headers. 
weird. */
 #defineDEFDATALEN  ICMP6ECHOTMLEN
 #define MAXDATALEN MAXPACKETLEN - IP6LEN - ICMP6ECHOLEN
-#defineNROUTES 9   /* number of record route slots 
*/
 
 #defineA(bit)  rcvd_tbl[(bit)>>3]  /* identify byte in 
array */
 #defineB(bit)  (1 << ((bit) & 0x07))   /* identify bit in byte 
*/
@@ -143,7 +142,6 @@ struct payload {
 #defineF_INTERVAL  0x0002
 #defineF_PINGFILLED0x0008
 #defineF_QUIET 0x0010
-#defineF_RROUTE0x0020
 #defineF_SO_DEBUG  0x0040
 #defineF_VERBOSE   0x0100
 #define F_NODEADDR 0x0800
@@ -154,14 +152,11 @@ struct payload {
 #define F_FQDNOLD  0x2
 #define F_NIGROUP  0x4
 #define F_SUPTYPES 0x8
-#define F_NOMINMTU 0x10
 #define F_AUD_RECV 0x20
 #define F_AUD_MISS 0x40
 #define F_NOUSERDATA   (F_NODEADDR | F_FQDN | F_FQDNOLD | F_SUPTYPES)
 u_int options;
 
-#define IN6LEN sizeof(struct in6_addr)
-#define SA6LEN sizeof(struct sockaddr_in6)
 #define DUMMY_PORT 10101
 
 /*
@@ -224,7 +219,7 @@ void retransmit(void);
 voidonint(int);
 size_t  pingerlen(void);
 int pinger(void);
-const char *pr_addr(struct sockaddr *, int);
+const char *pr_addr(struct sockaddr *, socklen_t);
 voidpr_icmph(struct icmp6_hdr *, u_char *);
 voidpr_iph(struct ip6_hdr *);
 voidpr_suptypes(struct icmp6_nodeinfo *, size_t);
@@ -1165,7 +1160,7 @@ pr_pack(u_char *buf, int cc, struct msgh
int i;
int hoplim;
struct sockaddr *from;
-   int fromlen;
+   socklen_t fromlen;
u_char *cp = NULL, *dp, *end = buf + cc;
struct in6_pktinfo *pktinfo = NULL;
struct timespec ts, tp;
@@ -2197,7 +2192,7 @@ pr_iph(struct ip6_hdr *ip6)
  * a hostname.
  */
 const char *
-pr_addr(struct sockaddr *addr, int addrlen)
+pr_addr(struct sockaddr *addr, socklen_t addrlen)
 {
static char buf[NI_MAXHOST];
int flag = 0;

Re: Cache-Control for httpd

[Larry Hynes]

Tuesday Sep 15 2015, 13:49:27 (+0100), Manuel Giraud :


Questions:
   - Is there any interest?


I'm interested! :D

Apologies if you were looking for interest from httpd devs, I am merely
a humble user. I'm currently using relayd to add an expires header to my
httpd traffic. It's okay, but I would rather handle it from httpd if
possible.