the logic for only doing zero-fill for writable
segments is wrong. It looks to me like the ELF gABI specifies
zero-fill semantics for memsz filesz even for read-only segments.
On Sat, Oct 5, 2013 at 4:27 PM, Philip Guenther guent...@gmail.com wrote:
On Thu, 15 Aug 2013, Maxime Villard wrote
Hi,
I put here a bug among others:
Index: ssh-ed25519.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-ed25519.c,v
retrieving revision 1.4
diff -u -r1.4 ssh-ed25519.c
--- ssh-ed25519.c 24 Jun 2014 01:13:21 - 1.4
+++
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
Hi,
- - - src/usr.sbin/smtpd/smtpd.c l.1326
if (! bsnprintf(key, sizeof key,
profiling.imsg.%s.%s.%s,
proc_name(smtpd_process),
proc_name(p-proc),
imsg_to_str(imsg-hdr.type)));-- ';'
stat_set(key,
Hi,
- - - - sys/kern/exec_elf.c l.236 ~ 251252
Are my code scanner and me wrong, or 'bdiff' may not be
initialized ?
Hi,
is it normal that in some functions like
tc_ticktock(void)
{
static int count;
if (++count tc_tick)
return;
count = 0;
tc_windup();
}
the static variables are not
Le 08/07/2013 11:00, Franco Fichtner a écrit :
Hi Maxime,
On Jul 8, 2013, at 10:40 AM, Maxime Villard rusty...@gmx.fr wrote:
the static variables are not initialized?
Static variables are always zeroed when not specified otherwise.
Regards,
Franco
Ah, yes. I didn't know.
Hi,
as I did for NetBSD, here is a list of 14 potential bugs/errors
found by my code scanner in OpenBSD:
http://M00nBSD.net/e5ab5f6e59d6a0feb7d1a518acc8233d.html
I do not provide patches.
Hi,
- - - dev/softraid_aoe.c - - -
I don't know if it's a mistake, but at l.393 the function always
returns 1.
If you put that just to temporarily shut down the function, then you
should add a comment to explicitly tell to the stupid guys who might
think it's a mistake - like me - something like
Hi,
in src/sys/dev/pcmcia/if_ray.c, at l.1018:
- - - -
case SIOCG80211NWID:
RAY_DPRINTF((%s: ioctl: cmd SIOCG80211NWID\n, ifp-if_xname));
error = copyout(sc-sc_cnwid, ifr-ifr_data,
sizeof(sc-sc_cnwid));
break; --
#ifdef
Le 06/07/2013 17:42, Kenneth R Westerback a écrit :
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
Hi,
after playing with some ELF binaries, I figured out that error
codes received in errno do not always match with what is expected
in the kernel.
Actually, this function should return (error) instead of (ENOEXEC):
Index: exec_elf.c
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
;
}
} 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
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
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
Le 12/07/2013 20:24, Maxime Villard a écrit :
Hi,
as I did for NetBSD, here is a list of 14 potential bugs/errors
found by my code scanner in OpenBSD:
http://M00nBSD.net/e5ab5f6e59d6a0feb7d1a518acc8233d.html
I do not provide patches.
May I ask what's the status of the remaining
up, please
Le 28/08/2013 21:43, Maxime Villard a écrit :
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
Hi,
when the kernel loads an ELF binary, it will also load its interpreter.
The kernel checks the rights of the interpreter, that way:
if ((error = VOP_ACCESS(vp, VREAD, p-p_ucred, p)) != 0)
goto bad1;
It should check with VEXEC instead of VREAD. Interpreters get
Le 20/10/2013 16:53, Theo de Raadt a écrit :
when the kernel loads an ELF binary, it will also load its interpreter.
The kernel checks the rights of the interpreter, that way:
if ((error = VOP_ACCESS(vp, VREAD, p-p_ucred, p)) != 0)
goto bad1;
It should check with VEXEC
Le 20/10/2013 18:05, Theo de Raadt a écrit :
Le 20/10/2013 16:53, Theo de Raadt a écrit :
when the kernel loads an ELF binary, it will also load its interpreter.
The kernel checks the rights of the interpreter, that way:
if ((error = VOP_ACCESS(vp, VREAD, p-p_ucred, p)) != 0)
On 10/20/13 21:54, Theo de Raadt wrote:
Indeed, the interpreter is not passed to execve. That's why I used
'get executed'
instead of
'are executed'
though the difference might not be clear.
The kernel loads the interpreter, and the code of that interpreter
gets executed. So,
Le 21/10/2013 09:38, Theo de Raadt a écrit :
I don't get what's wrong with running execve on it. In all cases,
someone can load it through another executable.
Using ld.so does not imply execve'ing it.
If I have an interpreter that I chmod as exec-only, I want this
interpreter to be
Hi,
a thing I spotted some weeks ago
- - - usr.bin/ftp/ftp.c l.1090 - - -
d = 0;
do {
wr = write(fileno(fout), buf + d, rd);
if (wr == -1 errno == EPIPE)
break;
d += wr;
rd -= wr;
}
Le 06/10/2013 01:09, Kenneth R Westerback a écrit :
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++,
Le 05/10/2013 23:10, Todd C. Miller a écrit :
That looks reasonable to me. I can't think of a reason to not
return the proper errno values, especially as things like ENOMEM
are already documented for execve(2).
- todd
up...
Le 22/11/2013 17:48, Ted Unangst a écrit :
On Fri, Nov 22, 2013 at 10:09, Stuart Henderson wrote:
On 2013/11/22 07:25, Maxime Villard wrote:
If write() fails without EPIPE, d is decremented, and the function
keeps looping. If write() succeeds after several loops, d will be
negative
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 29 Nov 2013 21:17:49 -
@@ -1089,9
Hi,
when loading a linux binary, the kernel could leak MAXPATHLEN bytes.
Index: linux_exec.c
===
RCS file: /cvs/src/sys/compat/linux/linux_exec.c,v
retrieving revision 1.38
diff -u -r1.38 linux_exec.c
--- linux_exec.c3 Nov
I would like to end this thread. Here's a final patch:
Index: exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.93
diff -u -r1.93 exec_elf.c
--- exec_elf.c 4 Jul 2013 17:37:05 - 1.93
+++
Is there something wrong with this patch?
Le 29/11/2013 20:44, Maxime Villard a écrit :
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13
Hi,
I was reading linux_socket.c when I came across a bug in
linux_sendmsg().
Index: linux_socket.c
===
RCS file: /cvs/src/sys/compat/linux/linux_socket.c,v
retrieving revision 1.46
diff -u -r1.46 linux_socket.c
--- linux_socket.c
Le 14/12/2013 19:18, Ted Unangst a écrit :
On Sat, Dec 14, 2013 at 19:06, Maxime Villard wrote:
Hi,
I was reading linux_socket.c when I came across a bug in
linux_sendmsg().
At l.1252, if control == NULL, the function jumps to 'done' and 'level'
is checked while it hasn't been initialized
Le 20/04/2014 02:38, Peter Malone a écrit :
Hi,
I'm using OpenBSD 5.5. courier-imap-4.13 is in the ports tree and it's
quite a mess. I started looking at it today with the hope of just
replacing some of the malloc,strcat strcpy calls with asprintf, but it
became clear before long that
Hi,
== src/usr.bin/sendbug/sendbug.c ==
Tell me if I'm wrong, but in the main() function, we call
getenv() two times (l. 113 134) without holding the result
of the first call.
According to man getenv:
The string pointed to may be overwritten by a subsequent
call to getenv()
After the second
Hi,
I was looking at some openssh code when I spotted a mistake
in a function from auth.c:
static int
secure_filename(FILE *f, const char *file, struct passwd *pw,
char *err, size_t errlen)
{
char buf[MAXPATHLEN];
struct stat st;
/* check the open file to avoid races
Le 14/12/2012 06:27, Darren Tucker a écrit :
On Thu, Dec 13, 2012 at 07:31:46PM +0100, Maxime Villard wrote:
Hi,
I was looking at some openssh code when I spotted a mistake
applied, thanks.
Another trivial patch. Make a more detailed error message.
Or, we should use strlcpy().
Ok
Hi,
here are my small changes for pfctl.
1) There are cases where we could leak a file descriptor by
returning.
2) We don't need to check memory before freeing it, as
free() already does that.
3) Just replaced a snprintf() by strlcpy(), it's faster.
Ok/Comments ?
Index: pfctl.c
Hi,
I was looking at readlink syscall. There is the following function in
kern/vfs_syscalls.c:
int
doreadlinkat(struct proc *p, int fd, const char *path, char *buf,
size_t count, register_t *retval)
{
struct vnode *vp;
struct iovec aiov;
struct uio auio;
int
Le 24/01/2013 19:10, Matthew Dempsky a écrit :
On Thu, Jan 24, 2013 at 9:57 AM, Maxime Villard rusty...@gmx.fr wrote:
Hum here, if vp-v_type != VLNK, auio is untouched, but before returning
we use auio.uio_resid, which is not initialized. Is it?
Nice catch. We should probably move
Hi,
there is a huge bug in the tftp daemon.
-- /usr/src/usr.sbin/tftpd/tftpd.c --
In tftp_open() on line 870, the daemon checks for options
(OACK) and handle them through the oack() function. Then,
it frees and NULLs the variable client-options.
oack() - l.1390 - does some stuff, and if an error
description, I was expecting the patch to include a line
setting options to null after freeing it. Whatever else we do, shouldn't we
do that too?
On Fri, Mar 15, 2013 at 00:02, Maxime Villard wrote:
Hi,
there is a huge bug in the tftp daemon.
-- /usr/src/usr.sbin/tftpd/tftpd.c
Le 15/03/2013 12:17, David Gwynne a écrit :
On 15/03/2013, at 9:02 AM, Maxime Villard rusty...@gmx.fr wrote:
Hi,
there is a huge bug in the tftp daemon.
sure, but then there's also a huge bug in your fix by your own definition of
huge.
when oack fails it frees the client struct
Hi,
here is a patch to remove two useless variables in ntpd. Spotted
when recompiling with -Wextra.
Ok/Comments?
Index: client.c
===
RCS file: /cvs/src/usr.sbin/ntpd/client.c,v
retrieving revision 1.89
diff -u -p -r1.89 client.c
---
I have been making a code scanner for a while, and I wanted to test
a new rule, so I scanned sys/pci/drm. It found an uninitialized
variable in intel_ddi.c.
Quite simple, if !(val DDI_BUF_CTL_ENABLE) at l.1420, the variable
'wait' is not initialized at l.1432.
I guess I should send it upstream,
FWIW, it would be wise to propagate the fix to the stable branch(es).
I guess people use compat-linux.
Le 31/01/2015 00:33, Alexander Bluhm a écrit :
On Fri, Jan 30, 2015 at 03:57:12PM -0700, Todd C. Miller wrote:
On Fri, 30 Jan 2015 22:55:06 +0100, Alexander Bluhm wrote:
sosetopt() calls
Hi,
I put here a bug among others:
-- sys/net/pf_ioctl.c --
1027qs = pool_get(pf_queue_pl, PR_WAITOK|PR_LIMITFAIL|PR_ZERO);
if (qs == NULL) {
error = ENOMEM;
break;
Hi,
I put here a bug among others:
-- sys/compat/linux/linux_socket.c --
969 if (lsa.optval != NULL) {
m = m_get(M_WAIT, MT_SOOPTS);
error = copyin(lsa.optval, mtod(m, caddr_t), lsa.optlen);
if (error) {
Hi,
I put here a bug among others:
-- dev/sdmmc/sdmmc.c --
783 data = malloc(ucmd-c_datalen, M_TEMP,
M_WAITOK | M_CANFAIL);
if (data == NULL)
Hi,
I put here a bug among others:
- sys/dev/ic/aic6915.c -
386 MGETHDR(m, M_DONTWAIT, MT_DATA);
if (m == NULL) {
printf(%s: unable to allocate Tx mbuf\n,
and Maxime Villard
for providing the basis for one of the changes.
Hum. It curiously looks like the patch I sent here about two years
ago that nobody gave a shit about:
http://marc.info/?l=openbsd-techm=138703150604223w=2
- - - -
And don't you feel like there's an obvious bug
Hi,
I put here two bugs among others:
sys/dev/pci/hifn7751.c
2757
if (!(m0-m_flags M_EXT))
m_freem(m0);
len = MCLBYTES;
totlen -= len;
m0-m_pkthdr.len = m0-m_len = len;
mlast = m0;
Hi,
I put here a bug among others:
-- netinet/ip_icmp.c --
925 rt = rtalloc(sintosa(sin), RT_REPORT|RT_RESOLVE, rtableid);
if (rt == NULL)
return (NULL);
/* Check if the route is actually usable */
if
Hi,
I put here a bug among others:
sys/dev/ipmi.c
1046buf = malloc(maxlen + 3, M_DEVBUF, M_NOWAIT);
if (buf == NULL) {
printf(%s: ipmi_recvcmd: malloc fails\n, DEVNAME(sc));
return (-1);
Hi,
I put here a bug among others:
--- sys/dev/rasops/rasops.c
1167f = malloc(sizeof(struct rotatedfont), M_DEVBUF, M_WAITOK);
if ((ncookie = wsfont_rotate(*cookie)) == -1)
return;
Hi,
I put here a bug among others:
--- sys/dev/ic/an.c
1030if (an_write_rid(sc, AN_RID_SSIDLIST, sc-sc_buf,
sizeof(sc-sc_buf.sc_ssidlist)) != 0) {
printf(%s: failed to write ssid list\n, ifp-if_xname);
Hi,
I put here a bug among others:
sys/dev/ic/ti.c ---
648 MGETHDR(m_new, M_DONTWAIT, MT_DATA);
if (m_new == NULL)
return (ENOBUFS);
m_new-m_len = m_new-m_pkthdr.len = MHLEN;
Hi,
I put here a bug among others:
sys/arch/vax/if/if_qe.c ---
146 ring = malloc(PROBESIZE, M_TEMP, M_WAITOK | M_ZERO);
bzero(sc, sizeof(struct qe_softc));
sc-sc_iot = ua-ua_iot;
sc-sc_ioh = ua-ua_ioh;
sc-sc_dmat =
Hi,
I put here a bug among others:
-- sys/dev/pci/if_et.c -
1808if (m_defrag(m, M_DONTWAIT)) {
m_freem(m);
printf(%s: can't defrag TX mbuf\n,
Well, I guess I'll have to admit that I find your attitude extremely
disrespectful. But I don't tend to feel particularly offended by this
kind of things, so it probably does not matter.
Le 21/07/2015 12:31, sam a écrit :
On Tue, 21 Jul 2015 11:31:44 +0200
Maxime Villard m...@m00nbsd.net wrote
Hi,
I put here a bug among others:
-- sys/dev/pci/if_bnx.c
if ((status L2_FHDR_STATUS_L2_VLAN_TAG)
!(sc-rx_mode BNX_EMAC_RX_MODE_KEEP_VLAN_TAG)) {
#if NVLAN 0
Hi,
I put here a bug among others:
- sys/kern/kern_exec.c -
char *pathbuf = NULL;
[...]
pathbuf = pool_get(namei_pool, PR_WAITOK);
[...]
/* setup new registers and do misc. setup. */
if
Hi,
I put here a bug among others:
--- sys/arch/hppa64/dev/apic.c -
struct evcount *cnt;
struct apic_iv *aiv, *biv;
void *iv;
int irq = APIC_INT_IRQ(ih);
int line = APIC_INT_LINE(ih);
u_int32_t ent0;
Got some time tonight; nothing new, just emptying my list:
http://m00nbsd.net/e5ab5f6e59d6a0feb7d1a518acc8233d.html#Unsorted-2
Summary:
_17/ UNINITIALIZED VARIABLE: sys/netinet/if_ether.c rev1.165
_18/ UNINITIALIZED VARIABLE: sys/net80211/ieee80211_pae_output.c rev1.20
_19/
CVSROOT:/cvs
Module name:src
Changes by: morti...@cvs.openbsd.org2020/02/15 15:59:55
Modified files:
sys/arch/amd64/amd64: vmm.c
Log message:
Add bounds check on addresses passed from guests in pvclock.
Fixes an issue where a guest can write to host memory by
In vmm_update_pvclock():
6868pvclock_gpa = vcpu->vc_pvclock_system_gpa & 0xFFF0; <--
controlled by the guest
6869if (!pmap_extract(vm->vm_map->pmap, pvclock_gpa, _hpa))
6870return (EINVAL);
6871pvclock_ti = (void*) PMAP_DIRECT_MAP(pvclock_hpa);
6872
6873
66 matches
Mail list logo
