linux-kernel - Re: [PATCH v2 6/6] misc: fastrpc: Add support for compat ioctls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <864aaef3-8d60-3beb-ce20-a9f41f78a32f@linaro.org>
Date:   Wed, 12 Dec 2018 11:44:05 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     robh+dt@...nel.org, arnd@...db.de, mark.rutland@....com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        bjorn.andersson@...aro.org, linux-arm-msm@...r.kernel.org,
        bkumar@....qualcomm.com, thierry.escande@...aro.org
Subject: Re: [PATCH v2 6/6] misc: fastrpc: Add support for compat ioctls

Thanks for the comments,

On 12/12/18 10:59, Greg KH wrote:
>> This patch adds support for compat ioctl from 32 bits userland to
>> Qualcomm fastrpc driver.
> Ick, why?
> 
> Why not just fix up your ioctl structures to not need that at all?  For
> new code being added to the kernel, there's no excuse to have to have
> compat structures anymore, just make your api sane and all should be
> fine.

Yes, I did try that after comments from Arnd,


> 
> What prevents you from doing that and requiring compat support?
> 
I removed most of the compat IOCTLS except this one.
The reason is that this ioctl takes arguments which can vary in number 
for each call. So args are passed as pointer to structure, rather than 
fixed size. I could not find better way to rearrange this to give a 
fixed size data structure. In theory number of arguments can vary from 
0-255 for both in & out.

current data structure looks like this:

struct fastrpc_invoke_args {
	__s32 fd;
	size_t length;
	void *ptr;
};

struct fastrpc_invoke {
	__u32 handle;
	__u32 sc;
	struct fastrpc_invoke_args *args;
};

Other option is to split this IOCTL into 3 parts
SET_INVOKE_METHOD, SET_INVOKE_ARGS and INVOKE
with some kinda handle to bind these three.

with below structures:
struct fastrpc_invoke {
	__u32 handle;
	__u32 sc;
};

struct fastrpc_invoke_arg {
	__s32 fd;
	__u64 length;
	__u64 ptr;
};
I can try this and see how this works before I send next version of patch!


--srini
> thanks,
> 
> greg k-h