| 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
| ||
|
Message-ID: <569696CE.1000707@felipetonello.com>
Date: Wed, 13 Jan 2016 18:26:22 +0000
From: Felipe Ferreri Tonello <eu@...ipetonello.com>
To: Michal Nazarewicz <mina86@...a86.com>, Felipe Balbi <balbi@...com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dan Carpenter <dan.carpenter@...cle.com>
Cc: Robert Baldyga <r.baldyga@...sung.com>,
Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
Pawel Szewczyk <p.szewczyk@...sung.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 1/2] usb: gadget: f_midi: use flexible array member for
gmidi_in_port elements
Hi Michael,
On 09/01/16 03:47, Michal Nazarewicz wrote:
> Reduce number of allocations, simplify memory management and reduce
> memory usage by stacking the gmidi_in_port elements at the end of the
> f_midi structure using a flexible array.
>
> Also, observe that gmidi_in_port::midi pointer is *never* used for any
> purpose so it can be safely removed.
>
> Signed-off-by: Michal Nazarewicz <mina86@...a86.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 43 ++++++++++++------------------------
> 1 file changed, 14 insertions(+), 29 deletions(-)
>
> On Tue, Jan 05 2016, Dan Carpenter wrote:
>> On Tue, Jan 05, 2016 at 02:55:31PM +0100, Michal Nazarewicz wrote:
>>> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
>>> {
>>> struct f_midi *midi = substream->rmidi->private_data;
>>>
>>> - if (!midi->in_port[substream->number])
>>> + if (substream->number > midi->in_ports)
>>
>> This is off by one. It should be >= midi->in_ports.
>
> Fixed.
>
> Since there were no objections to this small patch set (i.e. there was
> mostly silence about it), I’ll go ahead and interpret it as ‘go ahead
> and properly submit it’. :)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 898a570..fd67af2 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -55,7 +55,6 @@ static const char f_midi_longname[] = "MIDI Gadget";
> * USB <- IN endpoint <- rawmidi
> */
> struct gmidi_in_port {
> - struct f_midi *midi;
This change is unrelated. I sent a patch removing this pointer as well.
> int active;
> uint8_t cable;
> uint8_t state;
> @@ -78,7 +77,6 @@ struct f_midi {
>
> struct snd_rawmidi_substream *in_substream[MAX_PORTS];
> struct snd_rawmidi_substream *out_substream[MAX_PORTS];
> - struct gmidi_in_port *in_port[MAX_PORTS];
>
> unsigned long out_triggered;
> struct tasklet_struct tasklet;
> @@ -87,6 +85,8 @@ struct f_midi {
> int index;
> char *id;
> unsigned int buflen, qlen;
> +
> + struct gmidi_in_port in_ports_array[/* in_ports */];
> };
>
> static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -529,11 +529,11 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> req->length = 0;
> req->complete = f_midi_complete;
>
> - for (i = 0; i < MAX_PORTS; i++) {
> - struct gmidi_in_port *port = midi->in_port[i];
> + for (i = 0; i < midi->in_ports; i++) {
> + struct gmidi_in_port *port = midi->in_ports_array + i;
> struct snd_rawmidi_substream *substream = midi->in_substream[i];
>
> - if (!port || !port->active || !substream)
> + if (!port->active || !substream)
> continue;
>
> while (req->length + 3 < midi->buflen) {
> @@ -568,12 +568,12 @@ static int f_midi_in_open(struct snd_rawmidi_substream *substream)
> {
> struct f_midi *midi = substream->rmidi->private_data;
>
> - if (!midi->in_port[substream->number])
> + if (substream->number >= midi->in_ports)
> return -EINVAL;
>
> VDBG(midi, "%s()\n", __func__);
> midi->in_substream[substream->number] = substream;
> - midi->in_port[substream->number]->state = STATE_UNKNOWN;
> + midi->in_ports_array[substream->number].state = STATE_UNKNOWN;
> return 0;
> }
>
> @@ -589,11 +589,11 @@ static void f_midi_in_trigger(struct snd_rawmidi_substream *substream, int up)
> {
> struct f_midi *midi = substream->rmidi->private_data;
>
> - if (!midi->in_port[substream->number])
> + if (substream->number >= midi->in_ports)
> return;
>
> VDBG(midi, "%s() %d\n", __func__, up);
> - midi->in_port[substream->number]->active = up;
> + midi->in_ports_array[substream->number].active = up;
> if (up)
> tasklet_hi_schedule(&midi->tasklet);
> }
> @@ -1067,14 +1067,11 @@ static void f_midi_free(struct usb_function *f)
> {
> struct f_midi *midi;
> struct f_midi_opts *opts;
> - int i;
>
> midi = func_to_midi(f);
> opts = container_of(f->fi, struct f_midi_opts, func_inst);
> kfree(midi->id);
> mutex_lock(&opts->lock);
> - for (i = opts->in_ports - 1; i >= 0; --i)
> - kfree(midi->in_port[i]);
> kfree(midi);
> --opts->refcnt;
> mutex_unlock(&opts->lock);
> @@ -1115,26 +1112,16 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> }
>
> /* allocate and initialize one new instance */
> - midi = kzalloc(sizeof(*midi), GFP_KERNEL);
> + midi = kzalloc(
> + sizeof(*midi) + opts->in_ports * sizeof(*midi->in_ports_array),
> + GFP_KERNEL);
Is there a garantee that the compiler will always use in_ports_array at
the end of the allocated block?
> if (!midi) {
> mutex_unlock(&opts->lock);
> return ERR_PTR(-ENOMEM);
> }
>
> - for (i = 0; i < opts->in_ports; i++) {
> - struct gmidi_in_port *port = kzalloc(sizeof(*port), GFP_KERNEL);
> -
> - if (!port) {
> - status = -ENOMEM;
> - mutex_unlock(&opts->lock);
> - goto setup_fail;
> - }
> -
> - port->midi = midi;
> - port->active = 0;
> - port->cable = i;
> - midi->in_port[i] = port;
> - }
> + for (i = 0; i < opts->in_ports; i++)
> + midi->in_ports_array[i].cable = i;
>
> /* set up ALSA midi devices */
> midi->id = kstrdup(opts->id, GFP_KERNEL);
> @@ -1161,8 +1148,6 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
> return &midi->func;
>
> setup_fail:
> - for (--i; i >= 0; i--)
> - kfree(midi->in_port[i]);
> kfree(midi);
> return ERR_PTR(status);
> }
>
Felipe
Download attachment "0x92698E6A.asc" of type "application/pgp-keys" (7196 bytes)
Powered by blists - more mailing lists