Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Sep 2009 20:57:33 +0200
From:      =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no>
To:        Leunam Elebek <forensec@yahoo.de>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: Trouble with copyout, memcpy.... Plain-Text version =)
Message-ID:  <86ws3i3nj6.fsf@ds4.des.no>
In-Reply-To: <389605.70197.qm@web28503.mail.ukl.yahoo.com> (Leunam Elebek's message of "Mon, 28 Sep 2009 10:19:08 -0700 (PDT)")
References:  <389605.70197.qm@web28503.mail.ukl.yahoo.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Leunam Elebek <forensec@yahoo.de> writes:
>         /* The same as in user-space... */
>         size =3D sizeof(*info) * kit.k_nkits;
>         info =3D malloc(sz, M_DAQ, M_NOWAIT | M_ZERO);

You shouldn't use M_NOWAIT unless there is absolutely no way around it.

>         if (info =3D=3D NULL)
>                 ....

Unnecessary if you use M_WAITOK instead of M_NOWAIT.

>         /*
>          * Here I want to copy struct info from kernel to user-space.
>          * If i use memcpy, the result is that the system hangs
>          * and I need to reboot the machine. OK, I thought
>          * copyout() should be able to do the job for me...
>          */
>          if (copyout(info, arg, sz))

Nope, ioctl() takes care of the copyin() / copyout().  At this point,
arg is a pointer to a malloc()ed buffer of the right size (as specified
by the definition of DAQ_KITINFO).

>                  /*
>                   * Fuc[k-k] i still come inside this block. I always
>                   * get an EFAULT error.=20
>                   */

This means that either a) info doesn't point where you think it does, b)
arg doesn't point where you think it does, or c) sz doesn't have the
value you think it does.

In this case, it's a combination of the latter two: arg points to a
kernel buffer, so the use of copyout(9) is inappropriate, but in
addition, the size of that buffer is sizeof(daq_kitinfo), and you're
trying to copy far more.  You need to rethink your interface: either
return only one struct daq_kitinfo per ioctl() call, or pass in a struct
that contains a pointer to a userland buffer and a length, or use
something else than ioctl(2).

option 2 would be something like:

struct daq_ioctl {
	struct daq_kitinfo *info;
	int nkits;
};

#define GRP		   'd'
#define DAQ_KITINFO	   _IOWR(GRP, 3, struct daq_ioctl)

static int
my_ioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flags, struct threa=
d *td)
{
	struct daq_ioctl *di =3D (struct daq_ioctl *)arg;
	struct daq_kitinfo *info;
	struct daq_kit kit;=20
	int nkits, ret;

	/* ... */

	nkits =3D (kit.k_nkits > di->nkits) ? di->nkits : kit.k_nkits;
	info =3D malloc(nkits * sizeof(struct daq_kitinfo))

	/* ... */

	ret =3D copyout(info, di->info, nkits * sizeof(struct daq_kitinfo));
	/* let userland know what it got */
	if (ret =3D=3D 0)
		di->nkits =3D nkits;
	return (ret);
}

DES
--=20
Dag-Erling Sm=C3=B8rgrav - des@des.no



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ws3i3nj6.fsf>