linux-kernel - Re: [PATCH 22/22] xlink-core: factorize xlink_ioctl function by creating sub-functions for each ioctl command

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: <3595d2e5e5de009730b3374bf65e96359c02392d.camel@perches.com>
Date:   Sun, 06 Dec 2020 19:05:34 -0800
From:   Joe Perches <joe@...ches.com>
To:     mgross@...ux.intel.com, markgross@...nel.org, arnd@...db.de,
        bp@...e.de, damien.lemoal@....com, dragan.cvetic@...inx.com,
        gregkh@...uxfoundation.org, corbet@....net,
        leonard.crestez@....com, palmerdabbelt@...gle.com,
        paul.walmsley@...ive.com, peng.fan@....com, robh+dt@...nel.org,
        shawnguo@...nel.org
Cc:     linux-kernel@...r.kernel.org, Seamus Kelly <seamus.kelly@...el.com>
Subject: Re: [PATCH 22/22] xlink-core: factorize xlink_ioctl function by
 creating sub-functions for each ioctl command

On Tue, 2020-12-01 at 14:35 -0800, mgross@...ux.intel.com wrote:
> From: Seamus Kelly <seamus.kelly@...el.com>
> 
> Refactor the too large IOCTL function to call helper functions.

This should not be sent as a known poor patch as patch 21 of 22
and then updated in patch 22 of 22 with a better style.

This should be sent in the as desired final form the first time
so that people don't give you useless notes.

> @@ -342,427 +323,84 @@ static int kmb_xlink_remove(struct platform_device *pdev)
>   * IOCTL function for User Space access to xlink kernel functions
>   *
>   */
> +int ioctl_connect(unsigned long arg);
>  
> 
>  static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  {
> -	struct xlink_handle		devh	= {0};
> -	struct xlinkopenchannel		op	= {0};
> -	struct xlinkwritedata		wr	= {0};
> -	struct xlinkreaddata		rd	= {0};
> -	struct xlinkreadtobuffer	rdtobuf = {0};
> -	struct xlinkconnect		con	= {0};
> -	struct xlinkrelease		rel	= {0};
> -	struct xlinkstartvpu		startvpu = {0};
> -	struct xlinkcallback		cb	= {0};
> -	struct xlinkgetdevicename	devn	= {0};
> -	struct xlinkgetdevicelist	devl	= {0};
> -	struct xlinkgetdevicestatus	devs	= {0};
> -	struct xlinkbootdevice		boot	= {0};
> -	struct xlinkresetdevice		res	= {0};
> -	struct xlinkdevmode		devm	= {0};
> -	struct xlinkregdevevent		regdevevent = {0};
> -	u32 sw_device_id_list[XLINK_MAX_DEVICE_LIST_SIZE];
> -	char name[XLINK_MAX_DEVICE_NAME_SIZE];
> -	int interface = NULL_INTERFACE;
> -	u32 device_status = 0;
> -	u32 num_devices = 0;
> -	u32 device_mode = 0;
> -	u32 num_events = 0;
> -	char filename[64];
> -	u32 *ev_list;
> -	u8 reladdr;
> -	u8 *rdaddr;
> -	u32 size;
>  	int rc;
>  
> 
>  	switch (cmd) {
>  	case XL_CONNECT:
> -		if (copy_from_user(&con, (void __user *)arg,
> -				   sizeof(struct xlinkconnect)))
> -			return -EFAULT;
> -		if (copy_from_user(&devh, (void __user *)con.handle,
> -				   sizeof(struct xlink_handle)))
> -			return -EFAULT;
> -		rc = xlink_connect(&devh);
> -		if (rc == X_LINK_SUCCESS) {
> -			if (copy_to_user((void __user *)con.handle,
> -					 &devh, sizeof(struct xlink_handle)))
> -				return -EFAULT;
> -		}
> -		if (copy_to_user((void __user *)con.return_code, (void *)&rc,
> -				 sizeof(rc)))
> -			return -EFAULT;
> +		rc = ioctl_connect(arg);
>  		break;

etc...