Re: errors in usage.c - libusbhid
Thank you mpi for the response.
I will try to answer to the best of my knowledge.
> Are you sure the name always contain an underscore? Can't it be
> "Button:Button42" ?
It depends. At the moment it is dictated by the default page
/usr/share/misc/usb_hid_usages and the name looks more like "Button:Button
42".
So if someone decides to create another table, he/she could decide to name
it
"Button:Button-42" instead.
However if that would occur other things would break as well, this because
of how the
table is loaded by hid_start().
The patch is made to handle lines defined as " * %99[^\n]". When a line
with spaces
is found, hid_start() will replace the spaces with "_".
> You're passing the string directly to sscanf(3), is that safe?
I was initially uncertain about the safety of sscanf, But when reading the
man-page I could
only identify scenarios using it to scan "%s" as unsafe if you don't supply
a width as this
could lead to buffer overflows.
> Don't you want to parse only unsigned numbers? Or is it possible to
> have "Button:Button_-3" ?
Yes. You are right, which has lead me to replace sscanf with strtonum
instead.
In that way I can do a simple check of input correctness.
To be honest regarding input, I'm not certain about how high values the
hid-specification
allows, it does seem in the cases I have been able to identify, that it is
possible to use
the first two bytes.
Thank you for the questions.
Index: usage.c
===
RCS file: /cvs/src/lib/libusbhid/usage.c,v
retrieving revision 1.16
diff -u -p -r1.16 usage.c
--- usage.c 8 Oct 2014 04:49:36 - 1.16
+++ usage.c 14 Jun 2018 21:56:30 -
@@ -265,8 +265,10 @@ int
hid_parse_usage_in_page(const char *name)
{
const char *sep;
+ const char *usage_sep;
unsigned int l;
- int k, j;
+ int k, j, us, parsed_usage;
+ const char *errstr;
sep = strchr(name, ':');
if (sep == NULL)
@@ -278,9 +280,21 @@ hid_parse_usage_in_page(const char *name
return -1;
found:
sep++;
- for (j = 0; j < pages[k].pagesize; j++)
+ for (j = 0; j < pages[k].pagesize; j++) {
+ us = pages[k].page_contents[j].usage;
+ if (us == -1) {
+ usage_sep = strchr(sep, '_');
+ if (usage_sep == NULL)
+ return -1;
+ usage_sep++;
+ parsed_usage = strtonum(usage_sep, 0x1, 0x,
);
+ if (errstr == NULL)
+ return (pages[k].usage << 16) |
+ parsed_usage;
+ }
if (strcmp(pages[k].page_contents[j].name, sep) == 0)
return (pages[k].usage << 16) |
pages[k].page_contents[j].usage;
+ }
return -1;
}
2018-06-14 14:54 GMT+02:00 Martin Pieuchot :
> On 29/05/18(Tue) 22:29, David Bern wrote:
> > Sorry for the spamming.
> > After some research and finding that my fix for issue nr: 2 (
> > hid_usage_in_page() )
> > will break the functionality inside /usr.bin/usbhidaction/usbhidaction.c
> > https://goo.gl/1cWFtR (link to usbhidaction.c)
> >
> > I now change my patch to only include a fix for issue nr: 1
> > More details is described in the previous mail
>
> Are you sure the name always contain an underscore? Can't it be
> "Button:Button42" ?
>
> You're passing the string directly to sscanf(3), is that safe?
>
> Don't you want to parse only unsigned numbers? Or is it possible to
> have "Button:Button_-3" ?
>
> > Index: usage.c
> > ===
> > RCS file: /cvs/src/lib/libusbhid/usage.c,v
> > retrieving revision 1.16
> > diff -u -p -r1.16 usage.c
> > --- usage.c 8 Oct 2014 04:49:36 - 1.16
> > +++ usage.c 29 May 2018 19:45:25 -
> > @@ -265,8 +265,9 @@ int
> > hid_parse_usage_in_page(const char *name)
> > {
> > const char *sep;
> > + const char *usage_sep;
> > unsigned int l;
> > - int k, j;
> > + int k, j, us, parsed_usage;
> >
> > sep = strchr(name, ':');
> > if (sep == NULL)
> > @@ -278,9 +279,19 @@ hid_parse_usage_in_page(const char *name
> > return -1;
> > found:
> > sep++;
> > - for (j = 0; j < pages[k].pagesize; j++)
> > + for (j = 0; j < pages[k].pagesize; j++) {
> > + us = pages[k].page_contents[j].usage;
> > + if (us == -1) {
> > + usage_sep = strchr(sep, '_');
> > + if (usage_sep == NULL)
> > + return -1;
> > + if (sscanf(usage_sep, "_%d", _usage))
> > + return (pages[k].usage << 16) |
> > + parsed_usage;
> > + }
> > if
Re: Change CMakeLists.txt in LibreSSL to use target_include_directores
I propose some changes in this diff. I move more of cmake calls to target specific calls (target_sources, target_compile_definitions) to clean up the files and reduce their scope. On 14 Jun 2018, at 04:24, Brent Cook mailto:bust...@gmail.com>> wrote: You're correct, include/compat is intended to be private. We will need to make some tweaks here. On Mon, Jun 4, 2018 at 5:36 PM, Cameron Palmer mailto:came...@promon.no>> wrote: Question about the PUBLIC status of the ../include/compat headers in CMakeLists.txt. I wrote the target_include_directories calls to include ../include/compat in each of the targets and marked them PUBLIC, but I’m wondering if that will cause conflicts with system headers like time.h and if they should be marked PRIVATE. With them marked PUBLIC and including ssl or crypto one must add a compiler define like -D HAVE_CLOCK_GETTIME in the linking project to avoid a conflict. > On 29 May 2018, at 12:48, Brent Cook > mailto:bust...@gmail.com>> wrote: > > On Thu, May 24, 2018 at 10:10:58AM +, Cameron Palmer wrote: >> It is beneficial for projects that depend on LibreSSL libraries and are >> built with CMake to use target_link_libraries and automatically receive the >> PUBLIC or INTERFACE headers without needing to specify include_directories. >> This patch changes the project to use target_include_directories and header >> scoping. >> > > Makes sense. I made some minor fixes and committed to master. target_specific.diff Description: target_specific.diff
Re: [patch] Fix inaccurate comment in usr.bin/w/w.c
On Thu, Jun 14, 2018 at 06:28:47AM BST, Nan Xiao wrote: > Hi tech@, > > The following patch fix some inaccurate comment in w.c. E.g., there is > no "-n" option, and "-a" instead. Sorry id I am wrong, thanks! > > Index: w.c > === > RCS file: /cvs/src/usr.bin/w/w.c,v > retrieving revision 1.65 > diff -u -p -r1.65 w.c > --- w.c 18 Dec 2017 05:51:53 - 1.65 > +++ w.c 14 Jun 2018 05:17:00 - > @@ -71,9 +71,9 @@ struct winsize ws; > kvm_t *kd; > time_t now;/* the current time of day */ > int ttywidth; /* width of tty */ > -int argwidth; /* width of tty */ > -int header = 1; /* true if -h flag: don't print heading */ > -int nflag = 1; /* true if -n flag: don't convert addrs */ > +int argwidth; /* width of name and args of the current > process */ > +int header = 1; /* false if -h or -M flag: don't print heading > */ > +int nflag = 1; /* false if -a flag: don't convert addrs */ > int sortidle; /* sort by idle time */ > char*sel_user; /* login of particular user selected */ > char domain[HOST_NAME_MAX+1]; FYI, the '-n' to '-a' change happened nearly 22 years ago[0]. Given that "-a flag" should clearly be in the comment, shouldn't there a mechanical nflag -> aflag change also take place? [0] http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/w/w.c.diff?r1=1.5=1.6=h Regards, Raf
Re: Timeouting the pkg_add
On Thu, Jun 14, 2018 at 01:40:28PM -0400, sven falempin wrote: > Dear Tech Readers, > > In ftp command -w is available, very useful for crappy networks. > pkg_add does not have any of this. > > The HTTP layer is not configuring Timeout. > ( in /usr/libdata/perl5/OpenBSD/./PackageRepository/HTTP.pm ) > my $o = IO::Socket::INET->new( > PeerHost => $host, > PeerPort => $port); > ( plenty of option here like MultiHomed or Timeout, ReuseAddr, blocking ) That stuff is unused, disregard completly. > Maybe a patch can add a '-w' to pkg_add and push it there : Nope, use FETCH_CMD if necessary, and push whatever -w you need there. Note that this is documente
Re: Timeouting the pkg_add
On 2018/06/14 13:40, sven falempin wrote:
> Dear Tech Readers,
>
> In ftp command -w is available, very useful for crappy networks.
> pkg_add does not have any of this.
>
> The HTTP layer is not configuring Timeout.
> ( in /usr/libdata/perl5/OpenBSD/./PackageRepository/HTTP.pm )
> my $o = IO::Socket::INET->new(
> PeerHost => $host,
> PeerPort => $port);
> ( plenty of option here like MultiHomed or Timeout, ReuseAddr, blocking )
>
> Nor using an nonblocking Read later
>
> sub get_header
> {
> my $o = shift;
> my $l = $o->getline;
> if ($l !~ m,^HTTP/1\.1\s+(\d\d\d),) {
>
> Most usage may be used in FTP so none notice so far.
>
> SCP and FTP are done with the actual binary, ftp handle http well and
> is ready to get an option for crappy network.
>
> Maybe a patch can add a '-w' to pkg_add and push it there :
>
> sub ftp_cmd
> {
> my $self = shift;
> return $self->SUPER::ftp_cmd." -S
> session=/dev/fd/".fileno($self->{fh});
> }
>
> And use this for all transfer ?
>
> Or is the user supposed to manage this out of the pkg_add tool.
>
> I can propose a patch if not *
>
> Best.
>
> --
> --
> -
> Knowing is not enough; we must apply. Willing is not enough; we must do
>
I don't think it makes a lot of sense, if you know you're on a crappy network
that drops connection requests and would prefer to see failures quickly rather
than give it a better chance of actually connecting, you can just set
sysctl net.inet.tcp.keepinittime (the value is in half-seconds).
Timeouting the pkg_add
Dear Tech Readers,
In ftp command -w is available, very useful for crappy networks.
pkg_add does not have any of this.
The HTTP layer is not configuring Timeout.
( in /usr/libdata/perl5/OpenBSD/./PackageRepository/HTTP.pm )
my $o = IO::Socket::INET->new(
PeerHost => $host,
PeerPort => $port);
( plenty of option here like MultiHomed or Timeout, ReuseAddr, blocking )
Nor using an nonblocking Read later
sub get_header
{
my $o = shift;
my $l = $o->getline;
if ($l !~ m,^HTTP/1\.1\s+(\d\d\d),) {
Most usage may be used in FTP so none notice so far.
SCP and FTP are done with the actual binary, ftp handle http well and
is ready to get an option for crappy network.
Maybe a patch can add a '-w' to pkg_add and push it there :
sub ftp_cmd
{
my $self = shift;
return $self->SUPER::ftp_cmd." -S session=/dev/fd/".fileno($self->{fh});
}
And use this for all transfer ?
Or is the user supposed to manage this out of the pkg_add tool.
I can propose a patch if not *
Best.
--
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do
Re: Make witness(4) ready for UP systems
On Wed, Jun 13, 2018 at 10:45:08PM +0200, Christian Ludwig wrote: > It makes sense to have witness(4) on uniprocessor systems, too. Lock-order > violations are not an MP-only thing. Since UP kernels do not have the kernel > lock, wrap the code in appropriate ifdefs. Committed. Thank you.
OpenBSD Errata: June 14th, 2018 (libcrypto)
Errata patches for libcrypto have been released for OpenBSD 6.3 and 6.2. DSA and ECDSA signature generation can potentially leak secret information to a timing side-channel attack. Binary updates for the amd64, i386, and arm64 platforms are available via the syspatch utility. Source code patches can be found on the respective errata pages: https://www.openbsd.org/errata62.html https://www.openbsd.org/errata63.html For users running 6.3, the syspatches will be delayed approximately two days. Use the source code patch if you need the fix before then.
Re: ipmi(4): fix panic() with witness(4)
On 14/06/18(Thu) 22:24, Naoki Fukaumi wrote: > Hi tech@, > > ipmi(4) enabled -current kernel gets following panic() on boot, > > panic: acquiring blockable sleep lock with spinlock or critical section held > (kernel_lock) _lock @ /home/fukaumi/src/sys/arch/amd64/amd64/intr.c:525 > Stopped at db_enter+0x12: popq%r11 > TIDPIDUID PRFLAGS PFLAGS CPU COMMAND > *423398 6970 0 0x14000 0x2000 ipmicmd > db_enter(bd2a5f736e825eae,10,8000212b6b90,282,8,81561932) at > db_enter+0x12 > panic(20d,81b62719,81a3ba3b,81deb598,81b62718,81ccb464) > at panic+0x138 > witness_checkorder(a2fe973e3a67ed63,81a3ba3b,20d,0,81deb390,80087380) > at witness_checkorder+0xd52 > ___mp_lock(80087380,8000212b6d38,81c97ff0,0,817259d0,8000212b6ce8) > at ___mp_lock+0x70 > intr_handler(c1f8564e515d4217,4,8008c280,0,6,80087380) at > intr_handler+0x4c > Xintr_ioapic_edge21_untramp(0,0,0,0,0,34b00) at > Xintr_ioapic_edge21_untramp+0x13d > i82489_readreg(8f7f922366a4de4b,0,10,8000212b6e18,286,8) at > i82489_readreg+0x1a > lapic_delay(5ece2493000,2,81584942,8000212b > 6e50) at lapic_delay+0x72 > kcs_wait(e550ca8103c4e2f2,80093000,8000212b19d8,8008ac00,0,8f78bfa91f4d0kcs_wait+0x75 > kcs_sendmsg(ff3b00327acc0e2e,80093000,8000212b19d8,8000212b6f20,8114464e,8000212b6ed0) > at kcs_sendmsg+0xce > ipmi_cmd_poll(813fa090,8000212b19d8,811460ff,8000212b6ef0,ff3b00327acc0e2e,80093000) > at ipmi_cmd_poll+0x5f > ipmi_cmd_wait_cb(8008ac00,1,811462bf,8000212b6f10,813fa090,8000212b19d8) > at ipmi_cmd_wait_cb+0xf > taskq_thread(0,0,81889860,0,8000212b19d8,811462b0) at > taskq_thread+0x6d > end trace frame: 0x0, count: 2 > > > following diff fixes it. The problem is because you have an interrupt handler trying to grab the KERNEL_LOCK(). I'd suggest using IPL_MPFLOOR instead of IPL_NONE when initializing the mutex.
Re: route: replace hardcoded constants with defines
On Wed, Jun 13, 2018 at 09:31:00PM +0200, Klemens Nanni wrote:
> These seem more descriptive to me.
>
> No binary change.
>
> Feedback? OK?
OK bluhm@
> Index: route.c
> ===
> RCS file: /cvs/src/sbin/route/route.c,v
> retrieving revision 1.214
> diff -u -p -r1.214 route.c
> --- route.c 1 May 2018 18:14:10 - 1.214
> +++ route.c 13 Jun 2018 18:08:12 -
> @@ -754,13 +754,13 @@ inet_makenetandmask(u_int32_t net, struc
> else if (bits) {
> addr = net;
> mask = 0x << (32 - bits);
> - } else if (net < 128) {
> + } else if (net < IN_CLASSA_MAX) {
> addr = net << IN_CLASSA_NSHIFT;
> mask = IN_CLASSA_NET;
> - } else if (net < 65536) {
> + } else if (net < IN_CLASSB_MAX) {
> addr = net << IN_CLASSB_NSHIFT;
> mask = IN_CLASSB_NET;
> - } else if (net < 16777216L) {
> + } else if (net < (1 << 24)) {
> addr = net << IN_CLASSC_NSHIFT;
> mask = IN_CLASSC_NET;
> } else {
> @@ -1003,7 +1003,7 @@ getmplslabel(char *s, int in)
> const char *errstr;
> u_int32_t label;
>
> - label = strtonum(s, 0, 0x000f, );
> + label = strtonum(s, 0, MPLS_LABEL_MAX, );
> if (errstr)
> errx(1, "bad label: %s is %s", s, errstr);
> if (in) {
> @@ -1117,7 +1117,7 @@ rtmsg(int cmd, int flags, int fmask, uin
> cmd = RTM_CHANGE;
> else if (cmd == 'g') {
> cmd = RTM_GET;
> - if (so_ifp.sa.sa_family == 0) {
> + if (so_ifp.sa.sa_family == AF_UNSPEC) {
> so_ifp.sa.sa_family = AF_LINK;
> so_ifp.sa.sa_len = sizeof(struct sockaddr_dl);
> rtm_addrs |= RTA_IFP;
> @@ -1185,7 +1185,7 @@ mask_addr(union sockunion *addr, union s
> switch (addr->sa.sa_family) {
> case AF_INET:
> case AF_INET6:
> - case 0:
> + case AF_UNSPEC:
> return;
> }
> cp1 = mask->sa.sa_len + 1 + (char *)addr;
ipmi(4): fix panic() with witness(4)
Hi tech@,
ipmi(4) enabled -current kernel gets following panic() on boot,
panic: acquiring blockable sleep lock with spinlock or critical section held
(kernel_lock) _lock @ /home/fukaumi/src/sys/arch/amd64/amd64/intr.c:525
Stopped at db_enter+0x12: popq%r11
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
*423398 6970 0 0x14000 0x2000 ipmicmd
db_enter(bd2a5f736e825eae,10,8000212b6b90,282,8,81561932) at
db_enter+0x12
panic(20d,81b62719,81a3ba3b,81deb598,81b62718,81ccb464)
at panic+0x138
witness_checkorder(a2fe973e3a67ed63,81a3ba3b,20d,0,81deb390,80087380)
at witness_checkorder+0xd52
___mp_lock(80087380,8000212b6d38,81c97ff0,0,817259d0,8000212b6ce8)
at ___mp_lock+0x70
intr_handler(c1f8564e515d4217,4,8008c280,0,6,80087380) at
intr_handler+0x4c
Xintr_ioapic_edge21_untramp(0,0,0,0,0,34b00) at
Xintr_ioapic_edge21_untramp+0x13d
i82489_readreg(8f7f922366a4de4b,0,10,8000212b6e18,286,8) at
i82489_readreg+0x1a
lapic_delay(5ece2493000,2,81584942,8000212b
6e50) at lapic_delay+0x72
kcs_wait(e550ca8103c4e2f2,80093000,8000212b19d8,8008ac00,0,8f78bfa91f4d0kcs_wait+0x75
kcs_sendmsg(ff3b00327acc0e2e,80093000,8000212b19d8,8000212b6f20,8114464e,8000212b6ed0)
at kcs_sendmsg+0xce
ipmi_cmd_poll(813fa090,8000212b19d8,811460ff,8000212b6ef0,ff3b00327acc0e2e,80093000)
at ipmi_cmd_poll+0x5f
ipmi_cmd_wait_cb(8008ac00,1,811462bf,8000212b6f10,813fa090,8000212b19d8)
at ipmi_cmd_wait_cb+0xf
taskq_thread(0,0,81889860,0,8000212b19d8,811462b0) at
taskq_thread+0x6d
end trace frame: 0x0, count: 2
following diff fixes it.
Index: ipmi.c
===
RCS file: /cvs/src/sys/dev/ipmi.c,v
retrieving revision 1.101
diff -u -p -r1.101 ipmi.c
--- ipmi.c 13 Apr 2018 05:33:38 - 1.101
+++ ipmi.c 14 Jun 2018 13:02:55 -
@@ -1725,7 +1725,7 @@ ipmi_attach(struct device *parent, struc
c->c_sc = sc;
c->c_ccode = -1;
- sc->sc_cmd_taskq = taskq_create("ipmicmd", 1, IPL_NONE, TASKQ_MPSAFE);
+ sc->sc_cmd_taskq = taskq_create("ipmicmd", 1, IPL_NONE, 0);
mtx_init(>sc_cmd_mtx, IPL_NONE);
}
--
FUKAUMI Naoki
Re: errors in usage.c - libusbhid
On 29/05/18(Tue) 22:29, David Bern wrote:
> Sorry for the spamming.
> After some research and finding that my fix for issue nr: 2 (
> hid_usage_in_page() )
> will break the functionality inside /usr.bin/usbhidaction/usbhidaction.c
> https://goo.gl/1cWFtR (link to usbhidaction.c)
>
> I now change my patch to only include a fix for issue nr: 1
> More details is described in the previous mail
Are you sure the name always contain an underscore? Can't it be
"Button:Button42" ?
You're passing the string directly to sscanf(3), is that safe?
Don't you want to parse only unsigned numbers? Or is it possible to
have "Button:Button_-3" ?
> Index: usage.c
> ===
> RCS file: /cvs/src/lib/libusbhid/usage.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 usage.c
> --- usage.c 8 Oct 2014 04:49:36 - 1.16
> +++ usage.c 29 May 2018 19:45:25 -
> @@ -265,8 +265,9 @@ int
> hid_parse_usage_in_page(const char *name)
> {
> const char *sep;
> + const char *usage_sep;
> unsigned int l;
> - int k, j;
> + int k, j, us, parsed_usage;
>
> sep = strchr(name, ':');
> if (sep == NULL)
> @@ -278,9 +279,19 @@ hid_parse_usage_in_page(const char *name
> return -1;
> found:
> sep++;
> - for (j = 0; j < pages[k].pagesize; j++)
> + for (j = 0; j < pages[k].pagesize; j++) {
> + us = pages[k].page_contents[j].usage;
> + if (us == -1) {
> + usage_sep = strchr(sep, '_');
> + if (usage_sep == NULL)
> + return -1;
> + if (sscanf(usage_sep, "_%d", _usage))
> + return (pages[k].usage << 16) |
> + parsed_usage;
> + }
> if (strcmp(pages[k].page_contents[j].name, sep) == 0)
> return (pages[k].usage << 16) |
> pages[k].page_contents[j].usage;
> + }
> return -1;
> }
>
>
> comments? ok?
>
> 2018-05-28 13:01 GMT+02:00 David Bern :
>
> > I was suggested off list to give an explanation on what the patch does.
> >
> > So please, tell me if I need to clarify more, or make further changes to
> > the code.
> >
> > The patch tries to fix two things.
> > 1. Changes in hid_parse_usage_in_page() fixes problems in parsing usages
> > defined as: * Button %d
> >
> > hid_parse_usage_in_page():
> > Previously - With input "Button:Button_1" returns -1
> > Now - With input "Button:Button_1" returns 589825
> >
> > In the scenario of parsing Button:Button_1 we will not find a usage name
> > matching that string. For example Button:Button_1 is defined as
> > Button %d in the table.
> >
> > We are still able to calculate the proper usage number in the same way we
> > are
> > able to calculate the proper usage name in hid_usage_in_page().
> >
> > The first step is to identify if usage name is shortened. If it is,
> > usage will hold a value of -1. Then I try to locate a separator char in
> > the name as "_".
> > If a separator char is found I use it to read the value as "_%d" to get
> > the
> > corresponding usage number
> >
> > >+ us = pages[k].page_contents[j].usage;
> > >+ if (us == -1) {
> > >+ usage_sep = strchr(sep, '_');
> > >+ if (usage_sep == NULL)
> > >+ return -1;
> > >+ if (sscanf(usage_sep, "_%d", _usage))
> > >+ return (pages[k].usage << 16) |
> > >+ parsed_usage;
> >
> >
> > 2. The text-string that is returned by hid_usage_in_page() misses page
> > information.
> > So the changes made in hid_usage_in_page() is to make it the inverse of
> > hid_parse_usage_in_page()
> >
> > In details what the code previously did and now does.
> >
> > hid_usage_in_page():
> > Previously - With input 589825 returns Button_1
> > Now - With input 589825 returns Button:Button_1
> >
> >
> > The change just adds a pages[k].name to the string, a format that
> > hid_parse_usage_in_page() expects it to have.
> > I make formatting in two steps when us == -1 which it is when usage is
> > shortened
> > as for example: * Button %d.
> >
> > >+ snprintf(fmt, sizeof fmt,
> > >+ "%%s:%s", pages[k].page_contents[j].name);
> > >+ snprintf(b, sizeof b, fmt, pages[k].name, i);
> >
> > The first step is to create a format string that will result in something
> > like
> > "%s:Button_%d".
> > The last step I use the fmt-string to create a complete string that will
> > result in
> > "Button:Button_1"
> >
> >
> >
> >
> > 2018-05-24 18:44 GMT+02:00 David Bern :
> >
> >> While I was waiting for comments and feedback I came up with some
> >> improvements.
> >> The "logic" is
Re: fdinsert(), take 3
Martin Pieuchot wrote:
> This version should fixes the previous issue where LARVAL files were
> incorrectly accounted in find_last_set() resulting in a panic() in
> fd_unused().
>
> If you haven't read the previous explanations, the idea of this diff
> is to publish file descriptors only when they are completely setup.
> This will allow us to serialize access to file descriptors in a mp-safe
> way, which is the last requirement for unlock most of the socket-related
> syscalls.
>
> The important point of this diff is that we don't store on the descriptor
> itself the fact that it isn't ready yet (LARVAL). Otherwise we have a
> chicken & egg problem for reference counting. As pointed by Mathieu
> Masson other BSDs do that by using a new structure for open files. We
> might want to do that later when we'll turn these functions mp-safe :)
> For now, using the existing bitmask is good enough.
>
> Here are previous explanations:
> https://marc.info/?l=openbsd-tech=152579630904328=2
> https://marc.info/?l=openbsd-tech=152750590114899=2
>
> Please test & report as usual.
>
> Ok?
I've been running with basically the same diff and hitting hard on the
workload that would trigger the panic w/o the find_last_set fix and
couldn't reproduce it.
Mathieu.
>
> Index: sys/kern/exec_script.c
> ===
> RCS file: /cvs/src/sys/kern/exec_script.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 exec_script.c
> --- sys/kern/exec_script.c5 Jun 2018 09:29:05 - 1.46
> +++ sys/kern/exec_script.c14 Jun 2018 11:54:38 -
> @@ -170,17 +170,20 @@ check_shell:
> #endif
>
> fdplock(p->p_fd);
> - error = falloc(p, 0, , >ep_fd);
> - fdpunlock(p->p_fd);
> - if (error)
> + error = falloc(p, , >ep_fd);
> + if (error) {
> + fdpunlock(p->p_fd);
> goto fail;
> + }
>
> epp->ep_flags |= EXEC_HASFD;
> fp->f_type = DTYPE_VNODE;
> fp->f_ops =
> fp->f_data = (caddr_t) scriptvp;
> fp->f_flag = FREAD;
> - FILE_SET_MATURE(fp, p);
> + fdinsert(p->p_fd, epp->ep_fd, 0, fp);
> + fdpunlock(p->p_fd);
> + FRELE(fp, p);
> }
>
> /* set up the parameters for the recursive check_exec() call */
> Index: sys/kern/kern_descrip.c
> ===
> RCS file: /cvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 kern_descrip.c
> --- sys/kern/kern_descrip.c 5 Jun 2018 09:29:05 - 1.164
> +++ sys/kern/kern_descrip.c 14 Jun 2018 11:54:38 -
> @@ -73,6 +73,7 @@ int numfiles; /* actual number of open
> static __inline void fd_used(struct filedesc *, int);
> static __inline void fd_unused(struct filedesc *, int);
> static __inline int find_next_zero(u_int *, int, u_int);
> +static __inline int fd_inuse(struct filedesc *, int);
> int finishdup(struct proc *, struct file *, int, int, register_t *, int);
> int find_last_set(struct filedesc *, int);
> int dodup3(struct proc *, int, int, int, register_t *);
> @@ -125,7 +126,6 @@ int
> find_last_set(struct filedesc *fd, int last)
> {
> int off, i;
> - struct file **ofiles = fd->fd_ofiles;
> u_int *bitmap = fd->fd_lomap;
>
> off = (last - 1) >> NDENTRYSHIFT;
> @@ -139,11 +139,22 @@ find_last_set(struct filedesc *fd, int l
> if (i >= last)
> i = last - 1;
>
> - while (i > 0 && ofiles[i] == NULL)
> + while (i > 0 && !fd_inuse(fd, i))
> i--;
> return i;
> }
>
> +static __inline int
> +fd_inuse(struct filedesc *fdp, int fd)
> +{
> + u_int off = fd >> NDENTRYSHIFT;
> +
> + if (fdp->fd_lomap[off] & (1 << (fd & NDENTRYMASK)))
> + return 1;
> +
> + return 0;
> +}
> +
> static __inline void
> fd_used(struct filedesc *fdp, int fd)
> {
> @@ -190,7 +201,7 @@ fd_iterfile(struct file *fp, struct proc
> nfp = LIST_NEXT(fp, f_list);
>
> /* don't FREF when f_count == 0 to avoid race in fdrop() */
> - while (nfp != NULL && (nfp->f_count == 0 || !FILE_IS_USABLE(nfp)))
> + while (nfp != NULL && nfp->f_count == 0)
> nfp = LIST_NEXT(nfp, f_list);
> if (nfp != NULL)
> FREF(nfp);
> @@ -209,9 +220,6 @@ fd_getfile(struct filedesc *fdp, int fd)
> if ((u_int)fd >= fdp->fd_nfiles || (fp = fdp->fd_ofiles[fd]) == NULL)
> return (NULL);
>
> - if (!FILE_IS_USABLE(fp))
> - return (NULL);
> -
> FREF(fp);
> return (fp);
> }
> @@ -629,24 +637,24 @@ finishdup(struct proc *p, struct file *f
> struct filedesc *fdp = p->p_fd;
>
> fdpassertlocked(fdp);
> + KASSERT(fp->f_iflags & FIF_INSERTED);
> +
> if (fp->f_count == LONG_MAX-2) {
>
fdinsert(), take 3
This version should fixes the previous issue where LARVAL files were
incorrectly accounted in find_last_set() resulting in a panic() in
fd_unused().
If you haven't read the previous explanations, the idea of this diff
is to publish file descriptors only when they are completely setup.
This will allow us to serialize access to file descriptors in a mp-safe
way, which is the last requirement for unlock most of the socket-related
syscalls.
The important point of this diff is that we don't store on the descriptor
itself the fact that it isn't ready yet (LARVAL). Otherwise we have a
chicken & egg problem for reference counting. As pointed by Mathieu
Masson other BSDs do that by using a new structure for open files. We
might want to do that later when we'll turn these functions mp-safe :)
For now, using the existing bitmask is good enough.
Here are previous explanations:
https://marc.info/?l=openbsd-tech=152579630904328=2
https://marc.info/?l=openbsd-tech=152750590114899=2
Please test & report as usual.
Ok?
Index: sys/kern/exec_script.c
===
RCS file: /cvs/src/sys/kern/exec_script.c,v
retrieving revision 1.46
diff -u -p -r1.46 exec_script.c
--- sys/kern/exec_script.c 5 Jun 2018 09:29:05 - 1.46
+++ sys/kern/exec_script.c 14 Jun 2018 11:54:38 -
@@ -170,17 +170,20 @@ check_shell:
#endif
fdplock(p->p_fd);
- error = falloc(p, 0, , >ep_fd);
- fdpunlock(p->p_fd);
- if (error)
+ error = falloc(p, , >ep_fd);
+ if (error) {
+ fdpunlock(p->p_fd);
goto fail;
+ }
epp->ep_flags |= EXEC_HASFD;
fp->f_type = DTYPE_VNODE;
fp->f_ops =
fp->f_data = (caddr_t) scriptvp;
fp->f_flag = FREAD;
- FILE_SET_MATURE(fp, p);
+ fdinsert(p->p_fd, epp->ep_fd, 0, fp);
+ fdpunlock(p->p_fd);
+ FRELE(fp, p);
}
/* set up the parameters for the recursive check_exec() call */
Index: sys/kern/kern_descrip.c
===
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.164
diff -u -p -r1.164 kern_descrip.c
--- sys/kern/kern_descrip.c 5 Jun 2018 09:29:05 - 1.164
+++ sys/kern/kern_descrip.c 14 Jun 2018 11:54:38 -
@@ -73,6 +73,7 @@ int numfiles; /* actual number of open
static __inline void fd_used(struct filedesc *, int);
static __inline void fd_unused(struct filedesc *, int);
static __inline int find_next_zero(u_int *, int, u_int);
+static __inline int fd_inuse(struct filedesc *, int);
int finishdup(struct proc *, struct file *, int, int, register_t *, int);
int find_last_set(struct filedesc *, int);
int dodup3(struct proc *, int, int, int, register_t *);
@@ -125,7 +126,6 @@ int
find_last_set(struct filedesc *fd, int last)
{
int off, i;
- struct file **ofiles = fd->fd_ofiles;
u_int *bitmap = fd->fd_lomap;
off = (last - 1) >> NDENTRYSHIFT;
@@ -139,11 +139,22 @@ find_last_set(struct filedesc *fd, int l
if (i >= last)
i = last - 1;
- while (i > 0 && ofiles[i] == NULL)
+ while (i > 0 && !fd_inuse(fd, i))
i--;
return i;
}
+static __inline int
+fd_inuse(struct filedesc *fdp, int fd)
+{
+ u_int off = fd >> NDENTRYSHIFT;
+
+ if (fdp->fd_lomap[off] & (1 << (fd & NDENTRYMASK)))
+ return 1;
+
+ return 0;
+}
+
static __inline void
fd_used(struct filedesc *fdp, int fd)
{
@@ -190,7 +201,7 @@ fd_iterfile(struct file *fp, struct proc
nfp = LIST_NEXT(fp, f_list);
/* don't FREF when f_count == 0 to avoid race in fdrop() */
- while (nfp != NULL && (nfp->f_count == 0 || !FILE_IS_USABLE(nfp)))
+ while (nfp != NULL && nfp->f_count == 0)
nfp = LIST_NEXT(nfp, f_list);
if (nfp != NULL)
FREF(nfp);
@@ -209,9 +220,6 @@ fd_getfile(struct filedesc *fdp, int fd)
if ((u_int)fd >= fdp->fd_nfiles || (fp = fdp->fd_ofiles[fd]) == NULL)
return (NULL);
- if (!FILE_IS_USABLE(fp))
- return (NULL);
-
FREF(fp);
return (fp);
}
@@ -629,24 +637,24 @@ finishdup(struct proc *p, struct file *f
struct filedesc *fdp = p->p_fd;
fdpassertlocked(fdp);
+ KASSERT(fp->f_iflags & FIF_INSERTED);
+
if (fp->f_count == LONG_MAX-2) {
FRELE(fp, p);
return (EDEADLK);
}
- oldfp = fdp->fd_ofiles[new];
- if (oldfp != NULL) {
- if (!FILE_IS_USABLE(oldfp)) {
- FRELE(fp, p);
- return (EBUSY);
- }
- FREF(oldfp);
- }
+ oldfp = fd_getfile(fdp, new);
+ if
Re: tcp syn cache address family
On 14/06/18(Thu) 14:07, Alexander Bluhm wrote: > On Thu, Jun 14, 2018 at 10:59:30AM +0200, Claudio Jeker wrote: > > I'm not sure if I like this reachover from in_pcbconnect to > > in6_pcbconnect. It is a bit of a layer violation from the old times. > > Would be nice if those INET6 specifix reacovers could be removed from the > > low level INET functions in the long run. In this regard this feels a bit > > like a step back. > > I am a bit unsure in which direction to move. IMHO everything that reduces the IPv4 vs IPv6 ifdef maze in TCP/UDP/etc is the way to go. That's why I ok'ed your previous diff.
Re: tcp syn cache address family
On Thu, Jun 14, 2018 at 10:59:30AM +0200, Claudio Jeker wrote: > I'm not sure if I like this reachover from in_pcbconnect to > in6_pcbconnect. It is a bit of a layer violation from the old times. > Would be nice if those INET6 specifix reacovers could be removed from the > low level INET functions in the long run. In this regard this feels a bit > like a step back. I am a bit unsure in which direction to move. Having in_pcb... and in6_pcb... functions means code duplication. in_pcbbind() handles both. Often this is better as the v4 and v6 code cannot diverge. I if the requirements of v4 and v6 are different, distinct functions make sense. But that should not be exposed as API. Why should the caller care that in_pcbbind() is one function while in_pcbconnect() and in6_pcbconnect() are two? Would it be better to always call in_pcb...() and the implementation handles the details? In the old times the IPv4 code was copied to IPv6 and hacked until it worked. While this is a reasonable approach to achieve quick results, we should clean that up. There is more mess like in_pcblookup_local() which takes a void * address to handle both families. I tried to improve that but I am not statisfied with the result. In general I think we should move the switch(af) down, but be careful that the result is indeed better. bluhm
Re: fix file(1) memory leak
On Thu, Jun 14, 2018 at 08:18:22AM +0100, Nicholas Marriott wrote:
> I think this is better?
That works too.
ok brynet@
> Index: magic-test.c
> ===
> RCS file: /cvs/src/usr.bin/file/magic-test.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 magic-test.c
> --- magic-test.c 18 Apr 2017 14:16:48 - 1.25
> +++ magic-test.c 14 Jun 2018 07:16:41 -
> @@ -1404,10 +1404,10 @@ magic_test(struct magic *m, const void *
> if (*ms.out != '\0') {
> if (flags & MAGIC_TEST_MIME) {
> if (ms.mimetype != NULL)
> - return (xstrdup(ms.mimetype));
> + return (ms.mimetype);
> return (NULL);
> }
> - return (xstrdup(ms.out));
> + return (ms.out);
> }
> return (NULL);
> }
>
>
>
> On Wed, Jun 13, 2018 at 11:00:58PM -0400, Bryan Steele wrote:
> > magic_test returns a xstrdup'd string, which was then being xstrdup'd
> > again without freeing the original copy (leaking memory).
> >
> > casts added to avoid clang warning
> > warning: assigning to 'char *' from 'const char *' discards qualifiers
> > [-Wincompatible-pointer-types-discards-qualifiers]
> > inf->result = s;
> >
> > ok?
> >
> > -Bryan.
> >
> > Index: file/file.c
> > ===
> > RCS file: /cvs/src/usr.bin/file/file.c,v
> > retrieving revision 1.66
> > diff -u -p -u -r1.66 file.c
> > --- file/file.c 15 Jan 2018 19:45:51 - 1.66
> > +++ file/file.c 14 Jun 2018 02:55:32 -
> > @@ -603,7 +603,7 @@ try_text(struct input_file *inf)
> >
> > s = magic_test(inf->m, inf->base, inf->size, flags);
> > if (s != NULL) {
> > - inf->result = xstrdup(s);
> > + inf->result = (char *)s;
> > return (1);
> > }
> >
> > @@ -635,7 +635,7 @@ try_magic(struct input_file *inf)
> >
> > s = magic_test(inf->m, inf->base, inf->size, flags);
> > if (s != NULL) {
> > - inf->result = xstrdup(s);
> > + inf->result = (char *)s;
> > return (1);
> > }
> > return (0);
> >
>
Re: tcp syn cache address family
On Thu, Jun 14, 2018 at 10:30:19AM +0200, Alexander Bluhm wrote:
> On Thu, Jun 14, 2018 at 01:01:51AM +0200, Alexander Bluhm wrote:
> > - if (!bcmp(>sc_src, src, src->sa_len) &&
> > + if (sc->sc_src.sa.sa_family == src->sa_family &&
> > + sc->sc_dst.sa.sa_family == dst->sa_family &&
> > + !bcmp(>sc_src, src, src->sa_len) &&
> > !bcmp(>sc_dst, dst, dst->sa_len) &&
>
> Claudio pointed out that bcmp also checks sa_family.
>
> > case AF_INET:
> > - /* drop IPv4 packet to AF_INET6 socket */
> > - if (inp->inp_flags & INP_IPV6) {
> > - (void) m_free(am);
> > - goto resetandabort;
> > - }
>
> Here Claudio wants the additional check.
>
> What about this idea? in_pcbconnect() calls in6_pcbconnect() if
> necessary. The only check missing is INP_IPV6 in in6_pcbconnect().
> in_nam2sin() and in6_nam2sin6() care about sa_family. So we can
> simplify the code.
>
> ok?
I like this a lot better.
I'm not sure if I like this reachover from in_pcbconnect to
in6_pcbconnect. It is a bit of a layer violation from the old times.
Would be nice if those INET6 specifix reacovers could be removed from the
low level INET functions in the long run. In this regard this feels a bit
like a step back.
Why not leave the switch (src->sa_family)
but only call the in_pcbconnect or in6_pcbconnect there and let them
handle the errors via inX_nam2sinX()?
Then it may be possible to remove the #ifdef INET6 block in
in_pcbconnect() in a later step.
> bluhm
>
> Index: netinet/in_pcb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.237
> diff -u -p -r1.237 in_pcb.c
> --- netinet/in_pcb.c 11 Jun 2018 08:57:35 - 1.237
> +++ netinet/in_pcb.c 14 Jun 2018 08:18:36 -
> @@ -511,8 +511,7 @@ in_pcbconnect(struct inpcb *inp, struct
> #ifdef INET6
> if (sotopf(inp->inp_socket) == PF_INET6)
> return (in6_pcbconnect(inp, nam));
> - if ((inp->inp_flags & INP_IPV6) != 0)
> - panic("IPv6 pcb passed into in_pcbconnect");
> + KASSERT((inp->inp_flags & INP_IPV6) == 0);
> #endif /* INET6 */
>
> if ((error = in_nam2sin(nam, )))
> Index: netinet/tcp_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.356
> diff -u -p -r1.356 tcp_input.c
> --- netinet/tcp_input.c 11 Jun 2018 07:40:26 - 1.356
> +++ netinet/tcp_input.c 14 Jun 2018 08:21:15 -
> @@ -3537,27 +3537,9 @@ syn_cache_get(struct sockaddr *src, stru
> goto resetandabort;
> am->m_len = src->sa_len;
> memcpy(mtod(am, caddr_t), src, src->sa_len);
> -
> - switch (src->sa_family) {
> - case AF_INET:
> - /* drop IPv4 packet to AF_INET6 socket */
> - if (inp->inp_flags & INP_IPV6) {
> - (void) m_free(am);
> - goto resetandabort;
> - }
> - if (in_pcbconnect(inp, am)) {
> - (void) m_free(am);
> - goto resetandabort;
> - }
> - break;
> -#ifdef INET6
> - case AF_INET6:
> - if (in6_pcbconnect(inp, am)) {
> - (void) m_free(am);
> - goto resetandabort;
> - }
> - break;
> -#endif
> + if (in_pcbconnect(inp, am)) {
> + (void) m_free(am);
> + goto resetandabort;
> }
> (void) m_free(am);
>
> Index: netinet6/in6_pcb.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 in6_pcb.c
> --- netinet6/in6_pcb.c7 Jun 2018 08:46:24 - 1.103
> +++ netinet6/in6_pcb.c14 Jun 2018 08:19:10 -
> @@ -248,6 +248,8 @@ in6_pcbconnect(struct inpcb *inp, struct
> int error;
> struct sockaddr_in6 tmp;
>
> + KASSERT(inp->inp_flags & INP_IPV6);
> +
> if ((error = in6_nam2sin6(nam, )))
> return (error);
> if (sin6->sin6_port == 0)
>
--
:wq Claudio
Re: tcp syn cache address family
On Thu, Jun 14, 2018 at 01:01:51AM +0200, Alexander Bluhm wrote:
> - if (!bcmp(>sc_src, src, src->sa_len) &&
> + if (sc->sc_src.sa.sa_family == src->sa_family &&
> + sc->sc_dst.sa.sa_family == dst->sa_family &&
> + !bcmp(>sc_src, src, src->sa_len) &&
> !bcmp(>sc_dst, dst, dst->sa_len) &&
Claudio pointed out that bcmp also checks sa_family.
> case AF_INET:
> - /* drop IPv4 packet to AF_INET6 socket */
> - if (inp->inp_flags & INP_IPV6) {
> - (void) m_free(am);
> - goto resetandabort;
> - }
Here Claudio wants the additional check.
What about this idea? in_pcbconnect() calls in6_pcbconnect() if
necessary. The only check missing is INP_IPV6 in in6_pcbconnect().
in_nam2sin() and in6_nam2sin6() care about sa_family. So we can
simplify the code.
ok?
bluhm
Index: netinet/in_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.237
diff -u -p -r1.237 in_pcb.c
--- netinet/in_pcb.c11 Jun 2018 08:57:35 - 1.237
+++ netinet/in_pcb.c14 Jun 2018 08:18:36 -
@@ -511,8 +511,7 @@ in_pcbconnect(struct inpcb *inp, struct
#ifdef INET6
if (sotopf(inp->inp_socket) == PF_INET6)
return (in6_pcbconnect(inp, nam));
- if ((inp->inp_flags & INP_IPV6) != 0)
- panic("IPv6 pcb passed into in_pcbconnect");
+ KASSERT((inp->inp_flags & INP_IPV6) == 0);
#endif /* INET6 */
if ((error = in_nam2sin(nam, )))
Index: netinet/tcp_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.356
diff -u -p -r1.356 tcp_input.c
--- netinet/tcp_input.c 11 Jun 2018 07:40:26 - 1.356
+++ netinet/tcp_input.c 14 Jun 2018 08:21:15 -
@@ -3537,27 +3537,9 @@ syn_cache_get(struct sockaddr *src, stru
goto resetandabort;
am->m_len = src->sa_len;
memcpy(mtod(am, caddr_t), src, src->sa_len);
-
- switch (src->sa_family) {
- case AF_INET:
- /* drop IPv4 packet to AF_INET6 socket */
- if (inp->inp_flags & INP_IPV6) {
- (void) m_free(am);
- goto resetandabort;
- }
- if (in_pcbconnect(inp, am)) {
- (void) m_free(am);
- goto resetandabort;
- }
- break;
-#ifdef INET6
- case AF_INET6:
- if (in6_pcbconnect(inp, am)) {
- (void) m_free(am);
- goto resetandabort;
- }
- break;
-#endif
+ if (in_pcbconnect(inp, am)) {
+ (void) m_free(am);
+ goto resetandabort;
}
(void) m_free(am);
Index: netinet6/in6_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.103
diff -u -p -r1.103 in6_pcb.c
--- netinet6/in6_pcb.c 7 Jun 2018 08:46:24 - 1.103
+++ netinet6/in6_pcb.c 14 Jun 2018 08:19:10 -
@@ -248,6 +248,8 @@ in6_pcbconnect(struct inpcb *inp, struct
int error;
struct sockaddr_in6 tmp;
+ KASSERT(inp->inp_flags & INP_IPV6);
+
if ((error = in6_nam2sin6(nam, )))
return (error);
if (sin6->sin6_port == 0)
Re: fix file(1) memory leak
I think this is better?
Index: magic-test.c
===
RCS file: /cvs/src/usr.bin/file/magic-test.c,v
retrieving revision 1.25
diff -u -p -r1.25 magic-test.c
--- magic-test.c18 Apr 2017 14:16:48 - 1.25
+++ magic-test.c14 Jun 2018 07:16:41 -
@@ -1404,10 +1404,10 @@ magic_test(struct magic *m, const void *
if (*ms.out != '\0') {
if (flags & MAGIC_TEST_MIME) {
if (ms.mimetype != NULL)
- return (xstrdup(ms.mimetype));
+ return (ms.mimetype);
return (NULL);
}
- return (xstrdup(ms.out));
+ return (ms.out);
}
return (NULL);
}
On Wed, Jun 13, 2018 at 11:00:58PM -0400, Bryan Steele wrote:
> magic_test returns a xstrdup'd string, which was then being xstrdup'd
> again without freeing the original copy (leaking memory).
>
> casts added to avoid clang warning
> warning: assigning to 'char *' from 'const char *' discards qualifiers
> [-Wincompatible-pointer-types-discards-qualifiers]
> inf->result = s;
>
> ok?
>
> -Bryan.
>
> Index: file/file.c
> ===
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.66
> diff -u -p -u -r1.66 file.c
> --- file/file.c 15 Jan 2018 19:45:51 - 1.66
> +++ file/file.c 14 Jun 2018 02:55:32 -
> @@ -603,7 +603,7 @@ try_text(struct input_file *inf)
>
> s = magic_test(inf->m, inf->base, inf->size, flags);
> if (s != NULL) {
> - inf->result = xstrdup(s);
> + inf->result = (char *)s;
> return (1);
> }
>
> @@ -635,7 +635,7 @@ try_magic(struct input_file *inf)
>
> s = magic_test(inf->m, inf->base, inf->size, flags);
> if (s != NULL) {
> - inf->result = xstrdup(s);
> + inf->result = (char *)s;
> return (1);
> }
> return (0);
>
