net.inet.ip.forwarding=0 vs lo(4)
this is mostly about discussion at the moment, i'm not tied to this diff
at all.
the problem i'm hitting is that i have a multihomed box where the
service it provides listens on an IP address that's assigned to lo1.
it's a host running a service, it's not a router, so the
net.inet.ip.forwarding sysctl is not set to 1.
because of the checks introduced in src/sys/netinet/ip_input.c
r1.345, this doesnt work because a packet has to be received on the
interface the IP is assigned to. however, you cannot receive traffic on
an lo(4) interface (unless you're connecting from the local host), so
the addresses are on lo1 are not globally accessible.
i came up with this diff, which adds even more special casing for
loopback interfaces. it says addreesses on loopbacks are globally
reachable, even if ip forwarding is disabled.
is this reasonable? or is there a way i can do this without a diff
already? or is there a better diff to support this usecase? eg, would we
need a flag on IPs to specify if they're globally accessible or not?
also, i don't like the name "ip_laddr" for the function, but couldnt
come up with something better.
thoughts?
for those that are interested, the multihoming is via a bunch of gre(4)
interfaces so a set of frontend load balancers can route packets to
these backends, even if those backends are not directly connected to the
frontends because they're at different sites.
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.351
diff -u -p -r1.351 ip_input.c
--- netinet/ip_input.c 22 Aug 2020 17:55:30 - 1.351
+++ netinet/ip_input.c 16 Oct 2020 04:27:43 -
@@ -753,29 +753,42 @@ in_ouraddr(struct mbuf *m, struct ifnet
break;
}
}
- } else if (ipforwarding == 0 && rt->rt_ifidx != ifp->if_index &&
- !((ifp->if_flags & IFF_LOOPBACK) || (ifp->if_type == IFT_ENC) ||
- (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST))) {
- /* received on wrong interface. */
-#if NCARP > 0
- struct ifnet *out_if;
+ } else if (ipforwarding == 0 && !ip_laddr(ifp, m, rt)) {
+ ipstat_inc(ips_wrongif);
+ match = 2;
+ }
+
+ return (match);
+}
+int
+ip_laddr(struct ifnet *ifp, struct mbuf *m, struct rtentry *rt)
+{
+ struct ifnet *rtifp;
+ int match = 0;
+
+ if (rt->rt_ifidx == ifp->if_index ||
+ ifp->if_type == IFT_ENC ||
+ ISSET(ifp->if_flags, IFF_LOOPBACK) ||
+ ISSET(m->m_pkthdr.pf.flags, PF_TAG_TRANSLATE_LOCALHOST))
+ return (1);
+
+ /* received on a different interface. */
+ rtifp = if_get(rt->rt_ifidx);
+ if (rtifp != NULL) {
+ if (ISSET(rtifp->if_flags, IFF_LOOPBACK))
+ match = 1;
+#if NCARP > 0
/*
* Virtual IPs on carp interfaces need to be checked also
* against the parent interface and other carp interfaces
* sharing the same parent.
*/
- out_if = if_get(rt->rt_ifidx);
- if (!(out_if && carp_strict_addr_chk(out_if, ifp))) {
- ipstat_inc(ips_wrongif);
- match = 2;
- }
- if_put(out_if);
-#else
- ipstat_inc(ips_wrongif);
- match = 2;
+ else if (carp_strict_addr_chk(rtifp, ifp))
+ match = 1;
#endif
}
+ if_put(rtifp);
return (match);
}
Index: netinet/ip_var.h
===
RCS file: /cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.86
diff -u -p -r1.86 ip_var.h
--- netinet/ip_var.h8 Dec 2019 11:08:22 - 1.86
+++ netinet/ip_var.h16 Oct 2020 04:27:43 -
@@ -244,6 +244,7 @@ void ip_savecontrol(struct inpcb *, str
voidipintr(void);
int ip_input_if(struct mbuf **, int *, int, int, struct ifnet *);
int ip_deliver(struct mbuf **, int *, int, int);
+int ip_laddr(struct ifnet *, struct mbuf *, struct rtentry *);
voidip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
int rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
voidrip_init(void);
Index: netinet6/ip6_input.c
===
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.229
diff -u -p -r1.229 ip6_input.c
--- netinet6/ip6_input.c24 Aug 2020 16:40:07 - 1.229
+++ netinet6/ip6_input.c16 Oct 2020 04:27:43 -
@@ -425,30 +425,9 @@ ip6_input_if(struct mbuf **mp, int *offp
if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL)) {
struct in6_ifaddr *ia6 = ifatoia6(rt->rt_ifa);
- if (ip6_forwarding == 0 && rt->rt_ifidx !=
Re: push NET_LOCK() down in pf_ioctl.c
On Fri, Oct 16, 2020 at 11:37:22AM +0200, Alexandr Nedvedicky wrote:
> I've just found a forgotten diff in my tree. The diff pushes the NET_LCOK()
> further down in PF driver ioctl() path. The idea is to avoid sleeping while
> holding a NET_LOCK(). this typically may happen when we need to allocate
> memory. The diff is the first step as it takes care of easy/straightforward
> cases of such allocations. The allocations, which still may happen under
> the NET_LOCK() require more work in areas:
> PF tables,
> packet queues,
> transactions,
>
> the change is fairly large, but mostly mechanical.
Relocating malloc(9) and pool(9) seems good but other pf_*() calls are
now locked inconsistently after your diff.
Given that only reason about "allocations" this is either an oversight
and should be fixed or you need to provide more explanation.
See inline for the spots I'm talking about.
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index ef7d995e5a7..bac644fa6d1 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -1006,10 +1006,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> return (EACCES);
> }
>
> - NET_LOCK();
> switch (cmd) {
>
> case DIOCSTART:
> + NET_LOCK();
> PF_LOCK();
> if (pf_status.running)
> error = EEXIST;
> @@ -1025,9 +1025,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> DPFPRINTF(LOG_NOTICE, "pf: started");
> }
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
>
> case DIOCSTOP:
> + NET_LOCK();
> PF_LOCK();
> if (!pf_status.running)
> error = ENOENT;
> @@ -1038,6 +1040,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> DPFPRINTF(LOG_NOTICE, "pf: stopped");
> }
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
>
> case DIOCGETQUEUES: {
> @@ -1045,6 +1048,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> struct pf_queuespec *qs;
> u_int32_tnr = 0;
>
> + NET_LOCK();
> PF_LOCK();
> pq->ticket = pf_main_ruleset.rules.active.ticket;
>
> @@ -1056,6 +1060,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> }
> pq->nr = nr;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
>
> @@ -1064,10 +1069,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> struct pf_queuespec *qs;
> u_int32_tnr = 0;
>
> + NET_LOCK();
> PF_LOCK();
> if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
> error = EBUSY;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
>
> @@ -1078,10 +1085,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> if (qs == NULL) {
> error = EBUSY;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
> memcpy(>queue, qs, sizeof(pq->queue));
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
>
> @@ -1091,10 +1100,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> u_int32_tnr;
> int nbytes;
>
> + NET_LOCK();
> PF_LOCK();
> if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
> error = EBUSY;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
> nbytes = pq->nbytes;
> @@ -1107,6 +1118,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> if (qs == NULL) {
> error = EBUSY;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
> memcpy(>queue, qs, sizeof(pq->queue));
> @@ -1121,6 +1133,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
> struct proc *p)
> if (error == 0)
> pq->nbytes = nbytes;
> PF_UNLOCK();
> + NET_UNLOCK();
> break;
> }
>
> @@ -1128,38 +1141,44 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> struct pfioc_queue *q = (struct pfioc_queue *)addr;
> struct
Re: mixerctl names
On Sat, Oct 17, 2020 at 08:26:01PM +0200, Jan Stary wrote: > I balieve this is the purpose of outputs.master - it sets the volume > for multiple widgets; outputs.master.slaves says which. See azalia(4). > > > Now I just manually changed each inputs.dac alone. > > You are not supposed to tweak these controls directly; > that's mixerctl does, and it requires root privileges. > Try to set your controls with sndioctl as a regular user. > My laptop has amdgpu death syndrome. It will never leave 6.6-stable. It was cheap. You get what you pay for. Chris Bennett
Re: mixerctl names
On Oct 17 11:12:28, cpb_t...@bennettconstruction.us wrote: > On Sat, Oct 17, 2020 at 05:52:58PM +0200, Jan Stary wrote: > > Currently, mixerctl.conf(5) says > > > > Most devices have a number of digital to analogue converters > > (DACs), used for sound playback, and each DAC has a corresponding > > output mixer. The mixers are labelled “mix” or “sel”. > > > > That doesn't seem to be the case, at least not universaly > > as the wording seems to imply. For example, this is > > mixerctl output on a Thinkpad T400: > > > > inputs.dac-0:1=222,222 > > inputs.dac-2:3=222,222 > > inputs.beep=0 > > record.adc-2:3_source=mic2 > > record.adc-2:3=219,219 > > record.adc-0:1_source=mic > > record.adc-0:1=219,219 > > outputs.hp_source=dac-0:1 > > outputs.hp_boost=on > > inputs.mic=189,189 > > outputs.mic_dir=input-vr80 > > outputs.spkr_source=dac-2:3 > > outputs.spkr_eapd=on > > inputs.mic2=189,189 > > outputs.hp_sense=unplugged > > outputs.mic_sense=unplugged > > outputs.master=240,240 > > outputs.master.mute=off > > outputs.master.slaves= > > record.volume=240,240 > > record.volume.mute=off > > record.volume.slaves= > > record.enable=sysctl > > > > Apparently, it has two DACS (for the speakers and the headphones). > > The current wording might confuse the user into thinking he has > > no output mixer, but the > > > > inputs.dac-0:1=222,222 > > inputs.dac-2:3=222,222 > > > > do control the respective volumes, > > while no "mix" or "sel" exists. > > > > Similarly for recording via the two ADCs. > > > > > > Jan > > > Thank you! +1 > > I had no idea what was going on and had basically given up on having the > speakers off. > > changing outputs.master moves BOTH inputs.dac. Which is superbly > confusing! I balieve this is the purpose of outputs.master - it sets the volume for multiple widgets; outputs.master.slaves says which. See azalia(4). > Now I just manually changed each inputs.dac alone. You are not supposed to tweak these controls directly; that's mixerctl does, and it requires root privileges. Try to set your controls with sndioctl as a regular user.
Re: mixerctl names
On Sat, Oct 17, 2020 at 05:52:58PM +0200, Jan Stary wrote: > Currently, mixerctl.conf(5) says > > Most devices have a number of digital to analogue converters > (DACs), used for sound playback, and each DAC has a corresponding > output mixer. The mixers are labelled “mix” or “sel”. > > That doesn't seem to be the case, at least not universaly > as the wording seems to imply. For example, this is > mixerctl output on a Thinkpad T400: > > inputs.dac-0:1=222,222 > inputs.dac-2:3=222,222 > inputs.beep=0 > record.adc-2:3_source=mic2 > record.adc-2:3=219,219 > record.adc-0:1_source=mic > record.adc-0:1=219,219 > outputs.hp_source=dac-0:1 > outputs.hp_boost=on > inputs.mic=189,189 > outputs.mic_dir=input-vr80 > outputs.spkr_source=dac-2:3 > outputs.spkr_eapd=on > inputs.mic2=189,189 > outputs.hp_sense=unplugged > outputs.mic_sense=unplugged > outputs.master=240,240 > outputs.master.mute=off > outputs.master.slaves= > record.volume=240,240 > record.volume.mute=off > record.volume.slaves= > record.enable=sysctl > > Apparently, it has two DACS (for the speakers and the headphones). > The current wording might confuse the user into thinking he has > no output mixer, but the > > inputs.dac-0:1=222,222 > inputs.dac-2:3=222,222 > > do control the respective volumes, > while no "mix" or "sel" exists. > > Similarly for recording via the two ADCs. > > > Jan Thank you! +1 I had no idea what was going on and had basically given up on having the speakers off. changing outputs.master moves BOTH inputs.dac. Which is superbly confusing! Now I just manually changed each inputs.dac alone. Headphones plugged in -> speakers are off and headphones work. Unplug headphones -> speakers now turn on instead. I couldn't be happier. 8-} Chris Bennett
Re: [PATCH] Add USB Product ID for Logitech Webcam Pro 9000
On Sat, 17 Oct 2020 14:26:49 +0100 Raf Czlonka wrote: > Ping. It's committed, thanks. > On Sun, Oct 11, 2020 at 11:33:21AM BST, Raf Czlonka wrote: > > Hi all, > > > > I just dug a Logitech Webcam Pro 9000 (for Business) out. > > > > After a quick test, it seems to be working just fine but the > > Product ID isn't pretty-printed: > > > > $ usbdevs | grep 0x0809 > > addr 08: 046d:0809 Logitech, product 0x0809 > > > > lsusb confirms the Product ID: > > > > $ doas lsusb -v | grep 0x0809 > > idProduct 0x0809 Webcam Pro 9000 > > > > Regards, > > > > Raf > > > > Index: sys/dev/usb/usbdevs > > === > > RCS file: /cvs/src/sys/dev/usb/usbdevs,v > > retrieving revision 1.721 > > diff -u -p -r1.721 usbdevs > > --- sys/dev/usb/usbdevs 5 Oct 2020 05:28:13 - > > 1.721 +++ sys/dev/usb/usbdevs 11 Oct 2020 10:29:54 - > > @@ -2672,6 +2672,7 @@ product LOGITECH QUICKCAMWEB > > 0x0801 Quic product LOGITECH WEBCAMC2000x0802 > > Webcam C200 product LOGITECH WEBCAMC250 0x0804 Webcam > > C250 product LOGITECH WEBCAMC5000x0807 Webcam C500 > > +product LOGITECH WEBCAMPRO9000 0x0809 Webcam Pro 9000 > > product LOGITECH QUICKCAMPRO 0x0810 QuickCam Pro > > product LOGITECH WEBCAMC2100x0819 Webcam C210 > > product LOGITECH WEBCAMC3100x081b Webcam C310 >
mixerctl names
Currently, mixerctl.conf(5) says Most devices have a number of digital to analogue converters (DACs), used for sound playback, and each DAC has a corresponding output mixer. The mixers are labelled “mix” or “sel”. That doesn't seem to be the case, at least not universaly as the wording seems to imply. For example, this is mixerctl output on a Thinkpad T400: inputs.dac-0:1=222,222 inputs.dac-2:3=222,222 inputs.beep=0 record.adc-2:3_source=mic2 record.adc-2:3=219,219 record.adc-0:1_source=mic record.adc-0:1=219,219 outputs.hp_source=dac-0:1 outputs.hp_boost=on inputs.mic=189,189 outputs.mic_dir=input-vr80 outputs.spkr_source=dac-2:3 outputs.spkr_eapd=on inputs.mic2=189,189 outputs.hp_sense=unplugged outputs.mic_sense=unplugged outputs.master=240,240 outputs.master.mute=off outputs.master.slaves= record.volume=240,240 record.volume.mute=off record.volume.slaves= record.enable=sysctl Apparently, it has two DACS (for the speakers and the headphones). The current wording might confuse the user into thinking he has no output mixer, but the inputs.dac-0:1=222,222 inputs.dac-2:3=222,222 do control the respective volumes, while no "mix" or "sel" exists. Similarly for recording via the two ADCs. Jan Index: mixerctl.conf.5 === RCS file: /cvs/src/share/man/man5/mixerctl.conf.5,v retrieving revision 1.10 diff -u -p -r1.10 mixerctl.conf.5 --- mixerctl.conf.5 21 Apr 2020 21:32:26 - 1.10 +++ mixerctl.conf.5 17 Oct 2020 15:31:23 - @@ -66,10 +66,6 @@ TOSLink, RCA, or 1/8" mini stereo. Most devices have a number of digital to analogue converters (DACs), used for sound playback, and each DAC has a corresponding output mixer. -The mixers are labelled -.Dq mix -or -.Dq sel . Each DAC represents two channels of playback. .Pp Verify that playback works by playing an audio file @@ -104,10 +100,6 @@ shows the number of channels available. Most devices have a number of analogue to digital converters (ADCs), used for recording sound, and each ADC has a corresponding input mixer. -The mixers are labelled -.Dq mix -or -.Dq sel . Each ADC represents two channels of recording. .Pp Connect line in on the audio card to an audio source,
Re: drm: avoid possible deadlock in kthread_stop
> Date: Sat, 17 Oct 2020 16:16:01 +0200
> From: Sebastien Marie
>
> On Wed, Oct 14, 2020 at 08:58:04PM +0200, Mark Kettenis wrote:
> > > Date: Thu, 1 Oct 2020 09:09:50 +0200
> > > From: Sebastien Marie
> > >
> > > Hi,
> > >
> > > Currently, when a process is calling kthread_stop(), it sets a flag
> > > asking the thread to stop, and enters in sleep mode, but the code
> > > doing the stop doesn't wakeup the caller of kthread_stop().
> > >
> > > The thread should also be unparked as else it will not seen the
> > > KTHREAD_SHOULDSTOP flag. it follows what Linux is doing.
> > >
> > > While here, I added some comments in the locking logic for park/unpark
> > > and stop.
> > >
> > > Comments or OK ?
> >
> > I don't think adding all those comments makes a lot of sense. This
> > uses a fairly standard tsleep/wakeup pattern and the some of the
> > comments really state the obvious.
>
> it was the way I did to audit the code and understand what it did.
>
> > Can you do a diff that just adds
> > the missing wakeup() and kthread_unpark() call?
>
> here a new diff.
ok kettenis@
> diff 4efbe95c75086b3a7b0074651bfa04fd58990a98 /home/semarie/repos/openbsd/src
> blob - fd797effc74d6eb4a172c81be8feac0ed168ec5d
> file + sys/dev/pci/drm/drm_linux.c
> --- sys/dev/pci/drm/drm_linux.c
> +++ sys/dev/pci/drm/drm_linux.c
> @@ -207,6 +217,7 @@ kthread_func(void *arg)
>
> ret = thread->func(thread->data);
> thread->flags |= KTHREAD_STOPPED;
> + wakeup(thread);
> kthread_exit(ret);
> }
>
> @@ -298,8 +327,9 @@ kthread_stop(struct proc *p)
>
> while ((thread->flags & KTHREAD_STOPPED) == 0) {
> thread->flags |= KTHREAD_SHOULDSTOP;
> + kthread_unpark(p);
> wake_up_process(thread->proc);
> tsleep_nsec(thread, PPAUSE, "stop", INFSLP);
> }
> LIST_REMOVE(thread, next);
>
>
> Thanks.
> --
> Sebastien Marie
>
Re: drm: avoid possible deadlock in kthread_stop
On Wed, Oct 14, 2020 at 08:58:04PM +0200, Mark Kettenis wrote:
> > Date: Thu, 1 Oct 2020 09:09:50 +0200
> > From: Sebastien Marie
> >
> > Hi,
> >
> > Currently, when a process is calling kthread_stop(), it sets a flag
> > asking the thread to stop, and enters in sleep mode, but the code
> > doing the stop doesn't wakeup the caller of kthread_stop().
> >
> > The thread should also be unparked as else it will not seen the
> > KTHREAD_SHOULDSTOP flag. it follows what Linux is doing.
> >
> > While here, I added some comments in the locking logic for park/unpark
> > and stop.
> >
> > Comments or OK ?
>
> I don't think adding all those comments makes a lot of sense. This
> uses a fairly standard tsleep/wakeup pattern and the some of the
> comments really state the obvious.
it was the way I did to audit the code and understand what it did.
> Can you do a diff that just adds
> the missing wakeup() and kthread_unpark() call?
here a new diff.
diff 4efbe95c75086b3a7b0074651bfa04fd58990a98 /home/semarie/repos/openbsd/src
blob - fd797effc74d6eb4a172c81be8feac0ed168ec5d
file + sys/dev/pci/drm/drm_linux.c
--- sys/dev/pci/drm/drm_linux.c
+++ sys/dev/pci/drm/drm_linux.c
@@ -207,6 +217,7 @@ kthread_func(void *arg)
ret = thread->func(thread->data);
thread->flags |= KTHREAD_STOPPED;
+ wakeup(thread);
kthread_exit(ret);
}
@@ -298,8 +327,9 @@ kthread_stop(struct proc *p)
while ((thread->flags & KTHREAD_STOPPED) == 0) {
thread->flags |= KTHREAD_SHOULDSTOP;
+ kthread_unpark(p);
wake_up_process(thread->proc);
tsleep_nsec(thread, PPAUSE, "stop", INFSLP);
}
LIST_REMOVE(thread, next);
Thanks.
--
Sebastien Marie
Re: [PATCH] Add USB Product ID for Logitech Webcam Pro 9000
Ping. On Sun, Oct 11, 2020 at 11:33:21AM BST, Raf Czlonka wrote: > Hi all, > > I just dug a Logitech Webcam Pro 9000 (for Business) out. > > After a quick test, it seems to be working just fine but the > Product ID isn't pretty-printed: > > $ usbdevs | grep 0x0809 > addr 08: 046d:0809 Logitech, product 0x0809 > > lsusb confirms the Product ID: > > $ doas lsusb -v | grep 0x0809 > idProduct 0x0809 Webcam Pro 9000 > > Regards, > > Raf > > Index: sys/dev/usb/usbdevs > === > RCS file: /cvs/src/sys/dev/usb/usbdevs,v > retrieving revision 1.721 > diff -u -p -r1.721 usbdevs > --- sys/dev/usb/usbdevs 5 Oct 2020 05:28:13 - 1.721 > +++ sys/dev/usb/usbdevs 11 Oct 2020 10:29:54 - > @@ -2672,6 +2672,7 @@ product LOGITECH QUICKCAMWEB0x0801 Quic > product LOGITECH WEBCAMC200 0x0802 Webcam C200 > product LOGITECH WEBCAMC250 0x0804 Webcam C250 > product LOGITECH WEBCAMC500 0x0807 Webcam C500 > +product LOGITECH WEBCAMPRO9000 0x0809 Webcam Pro 9000 > product LOGITECH QUICKCAMPRO 0x0810 QuickCam Pro > product LOGITECH WEBCAMC210 0x0819 Webcam C210 > product LOGITECH WEBCAMC310 0x081b Webcam C310
httpd(8): server/location directory index question
Given the following httpd.conf snippet
server "example.com" {
...
directory no index
...
location "/foo" {
...
directory auto index
...
}
}
the URL http://example.com/foo surprisingly results in a 403
response.
With the directory option of the "/foo" location changed to
location "/foo" {
directory {
index "index.html"
auto index
}
}
the auto-index is being generated (as expected).
I was wondering if there is any reason why the 'auto index' of
the location shouldn't implicitly override the 'no index' option
of the enclosing server (as it happens with the patch below).
Index: usr.sbin/httpd/config.c
===
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.61
diff -u -p -u -p -r1.61 config.c
--- usr.sbin/httpd/config.c 21 Sep 2020 09:42:07 - 1.61
+++ usr.sbin/httpd/config.c 17 Oct 2020 12:26:20 -
@@ -488,11 +488,10 @@ config_getserver_config(struct httpd *en
if (srv_conf->flags & SRVFLAG_LOCATION) {
/* Inherit configuration from the parent */
f = SRVFLAG_INDEX|SRVFLAG_NO_INDEX;
- if ((srv_conf->flags & f) == 0) {
+ if ((srv_conf->flags & f) == 0)
srv_conf->flags |= parent->flags & f;
- (void)strlcpy(srv_conf->index, parent->index,
- sizeof(srv_conf->index));
- }
+ (void)strlcpy(srv_conf->index, parent->index,
+ sizeof(srv_conf->index));
f = SRVFLAG_AUTO_INDEX|SRVFLAG_NO_AUTO_INDEX;
if ((srv_conf->flags & f) == 0)
Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 parse.y
--- usr.sbin/httpd/parse.y 11 Oct 2020 03:21:44 - 1.118
+++ usr.sbin/httpd/parse.y 17 Oct 2020 12:26:20 -
@@ -1006,6 +1006,8 @@ dirflags : INDEX STRING {
srv_conf->flags |= SRVFLAG_NO_INDEX;
}
| AUTO INDEX{
+ srv_conf->flags &= ~SRVFLAG_NO_INDEX;
+ srv_conf->flags |= SRVFLAG_INDEX;
srv_conf->flags &= ~SRVFLAG_NO_AUTO_INDEX;
srv_conf->flags |= SRVFLAG_AUTO_INDEX;
}
Re: Non-const basename: usr.bin/ftp
On Thu, Oct 15 2020, Christian Weisgerber wrote:
> Accommodate POSIX basename(3) that takes a non-const parameter and
> may modify the string buffer.
>
> I've tried to follow the conventions of the existing code.
>
> ok?
>
> Index: usr.bin/ftp/fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.197
> diff -u -p -r1.197 fetch.c
> --- usr.bin/ftp/fetch.c 4 Jul 2020 11:23:35 - 1.197
> +++ usr.bin/ftp/fetch.c 15 Oct 2020 21:14:28 -
> @@ -192,7 +192,7 @@ file_get(const char *path, const char *o
> int fd, out = -1, rval = -1, save_errno;
> volatile sig_t oldintr, oldinti;
> const char *savefile;
> - char*buf = NULL, *cp;
> + char*buf = NULL, *cp, *pathbuf = NULL;
> const size_t buflen = 128 * 1024;
> off_thashbytes;
> ssize_t len, wlen;
> @@ -215,8 +215,12 @@ file_get(const char *path, const char *o
> else {
> if (path[strlen(path) - 1] == '/') /* Consider no file */
> savefile = NULL;/* after dir invalid. */
> - else
> - savefile = basename(path);
> + else {
> + pathbuf = strdup(path);
> + if (pathbuf == NULL)
> + errx(1, "Can't allocate memory for filename");
> + savefile = basename(pathbuf);
> + }
> }
>
> if (EMPTYSTRING(savefile)) {
> @@ -294,6 +298,7 @@ file_get(const char *path, const char *o
>
> cleanup_copy:
> free(buf);
> + free(pathbuf);
> if (out >= 0 && out != fileno(stdout))
> close(out);
> close(fd);
> @@ -315,6 +320,7 @@ url_get(const char *origline, const char
> int isunavail = 0, retryafter = -1;
> struct addrinfo hints, *res0, *res;
> const char *savefile;
> + char *pathbuf = NULL;
> char *proxyurl = NULL;
> char *credentials = NULL, *proxy_credentials = NULL;
> int fd = -1, out = -1;
> @@ -412,8 +418,12 @@ noslash:
> else {
> if (path[strlen(path) - 1] == '/') /* Consider no file */
> savefile = NULL;/* after dir invalid. */
> - else
> - savefile = basename(path);
> + else {
> + pathbuf = strdup(path);
> + if (pathbuf == NULL)
> + errx(1, "Can't allocate memory for filename");
> + savefile = basename(pathbuf);
> + }
> }
>
> if (EMPTYSTRING(savefile)) {
> @@ -1106,6 +1116,7 @@ cleanup_url_get:
> if (out >= 0 && out != fileno(stdout))
> close(out);
> free(buf);
> + free(pathbuf);
> free(proxyhost);
> free(proxyurl);
> free(newline);
> Index: usr.bin/ftp/util.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/util.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 util.c
> --- usr.bin/ftp/util.c6 Jul 2020 17:11:29 - 1.93
> +++ usr.bin/ftp/util.c15 Oct 2020 21:31:55 -
> @@ -763,7 +763,7 @@ progressmeter(int flag, const char *file
> off_t cursize, abbrevsize;
> double elapsed;
> int ratio, barlength, i, remaining, overhead = 30;
> - char buf[512];
> + char buf[512], *filenamebuf;
>
> if (flag == -1) {
> clock_gettime(CLOCK_MONOTONIC, );
> @@ -782,11 +782,13 @@ progressmeter(int flag, const char *file
> ratio = MAXIMUM(ratio, 0);
> ratio = MINIMUM(ratio, 100);
> if (!verbose && flag == -1) {
> - filename = basename(filename);
> - if (filename != NULL) {
> + filenamebuf = strdup(filename);
> + filename = basename(filenamebuf);
> + if (filenamebuf != NULL && filename != NULL) {
Can basename(3) handle a NULL input? Yes, but I had to check, and the
semantics in this case are weird. IMO it's better to do the error
checking in a more usual way, something like the diff below.
If you're fine with that, ok jca@
Index: util.c
===
RCS file: /d/cvs/src/usr.bin/ftp/util.c,v
retrieving revision 1.93
diff -u -p -p -u -r1.93 util.c
--- util.c 6 Jul 2020 17:11:29 - 1.93
+++ util.c 17 Oct 2020 12:20:27 -
@@ -763,7 +763,7 @@ progressmeter(int flag, const char *file
off_t cursize, abbrevsize;
double elapsed;
int ratio, barlength, i, remaining, overhead = 30;
- char buf[512];
+ char buf[512], *filenamebuf;
if (flag == -1) {
clock_gettime(CLOCK_MONOTONIC, );
@@ -782,11 +782,12 @@ progressmeter(int flag, const char *file
ratio = MAXIMUM(ratio, 0);
ratio =
