linux-kernel - Re: [PATCH 1/2] fs: configfs: make qw

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: <20170210111929.GA22347@kroah.com>
Date:   Fri, 10 Feb 2017 12:19:29 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Stefan Agner <stefan@...er.ch>
Cc:     balbi@...nel.org, andrzej.p@...sung.com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] fs: configfs: make qw_sign attribute symmetric

On Thu, Feb 09, 2017 at 10:04:43AM -0800, Stefan Agner wrote:
> On 2017-02-01 08:59, Stefan Agner wrote:
> > On 2017-02-01 00:06, Greg KH wrote:
> >> On Tue, Jan 31, 2017 at 06:19:16PM -0800, Stefan Agner wrote:
> >>> Currently qw_sign requires UTF-8 character to set, but returns UTF-16
> >>> when read. This isn't obvious when simply using cat since the null
> >>> characters are not visible, but hexdump unveils the true string:
> >>>
> >>>   # echo MSFT100 > os_desc/qw_sign
> >>>   # hexdump -C os_desc/qw_sign
> >>>   00000000  4d 00 53 00 46 00 54 00  31 00 30 00 30 00        |M.S.F.T.1.0.0.|
> >>>
> >>> Make qw_sign symmetric by returning an UTF-8 string too. Also follow
> >>> common convention and add a new line at the end.
> >>
> >> Doesn't USB require that strings be in UTF-16?  So why have the kernel
> >> convert them?
> > 
> > That is a discussion we should have had when the write side of this has
> > been added:
> > 
> > static ssize_t os_desc_qw_sign_store(struct config_item *item, const
> > char *page,
> > 				     size_t len)
> > {
> > 	struct gadget_info *gi = os_desc_item_to_gadget_info(item);
> > 	int res, l;
> > 
> > 	l = min((int)len, OS_STRING_QW_SIGN_LEN >> 1);
> > 	if (page[l - 1] == '\n')
> > 		--l;
> > 
> > 	mutex_lock(&gi->lock);
> > 	res = utf8s_to_utf16s(page, l,
> > 			      UTF16_LITTLE_ENDIAN, (wchar_t *) gi->qw_sign,
> > 			      OS_STRING_QW_SIGN_LEN);
> > 	if (res > 0)
> > 		res = len;
> > 	mutex_unlock(&gi->lock);
> > 
> > 	return res;
> > }
> > 
> > 
> > The store function is definitely already in use today, e.g. this script
> > used for ev3dev:
> > https://github.com/ev3dev/ev3-systemd/blob/ev3dev-jessie/scripts/ev3-usb.sh
> > 
> > Changing it to UTF-16 would break that script... So changing the store
> > part is the lesser of two evils.
> > 
> > Regarding new line: Just following what other attributes are doing by
> > using the GS_STRINGS_R macro.
> 
> Any comment on this? In my opinion especially this first patch really
> fixes a bug and should get applied... I can remove the newline if
> preferred.

It's up to Felipe, give him a chance to catch up on patches...

thanks,

greg k-h