From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751817AbeEESiT (ORCPT ); Sat, 5 May 2018 14:38:19 -0400 Received: from mta-p5.oit.umn.edu ([134.84.196.205]:58784 "EHLO mta-p5.oit.umn.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751252AbeEESiR (ORCPT ); Sat, 5 May 2018 14:38:17 -0400 X-Google-Smtp-Source: AB8JxZosxZm6QQc5S/RrO4Xt+no6yXqPSUC+0Ia4SIMivK9iCzq8qmKW3yx4qrDjsau5rJPhZx75iw== From: Wenwen Wang To: Wenwen Wang Cc: Kangjie Lu , Jaroslav Kysela , Takashi Iwai , alsa-devel@alsa-project.org (moderated list:SOUND), linux-kernel@vger.kernel.org (open list) Subject: [PATCH] ALSA: control: fix a redundant-copy issue Date: Sat, 5 May 2018 13:38:03 -0500 Message-Id: <1525545485-12183-1-git-send-email-wang6495@umn.edu> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org In snd_ctl_elem_add_compat(), the fields of the struct 'data' need to be copied from the corresponding fields of the struct 'data32' in userspace. This is achieved by invoking copy_from_user() and get_user() functions. The problem here is that the 'type' field is copied twice. One is by copy_from_user() and one is by get_user(). Given that the 'type' field is not used between the two copies, the second copy is *completely* redundant and should be removed for better performance and cleanup. Also, these two copies can cause inconsistent data: as the struct 'data32' resides in userspace and a malicious userspace process can race to change the 'type' field between the two copies to cause inconsistent data. Depending on how the data is used in the future, such an inconsistency may cause potential security risks. For above reasons, we should take out the second copy. Signed-off-by: Wenwen Wang --- sound/core/control_compat.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c index a848836..507fd52 100644 --- a/sound/core/control_compat.c +++ b/sound/core/control_compat.c @@ -396,8 +396,7 @@ static int snd_ctl_elem_add_compat(struct snd_ctl_file *file, if (copy_from_user(&data->id, &data32->id, sizeof(data->id)) || copy_from_user(&data->type, &data32->type, 3 * sizeof(u32))) goto error; - if (get_user(data->owner, &data32->owner) || - get_user(data->type, &data32->type)) + if (get_user(data->owner, &data32->owner)) goto error; switch (data->type) { case SNDRV_CTL_ELEM_TYPE_BOOLEAN: -- 2.7.4