Message ID | 193d36b0-961a-4b66-b945-37988f157ebe@moroto.mountain |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:bc8a:b0:106:860b:bbdd with SMTP id dn10csp717163dyb; Tue, 13 Feb 2024 10:10:50 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCW+XP5vKsmS6wt+AcytRs9p6Rc9Jsvy1zS+mZEHo1R13G7xKc0vUNT1ezAwqi7Pm9XNLwCCnpV7nQOmmWPJ96QRpSJgJQ== X-Google-Smtp-Source: AGHT+IG3XEiwd+3cqOmJ0Z1t4CTw/cRaVLn4vqdHCWyRqUYD+pKVBgntBeidaQLr2qCo+wurIGwq X-Received: by 2002:a0c:cc0c:0:b0:68d:1c2:6d56 with SMTP id r12-20020a0ccc0c000000b0068d01c26d56mr279818qvk.9.1707847850323; Tue, 13 Feb 2024 10:10:50 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707847850; cv=pass; d=google.com; s=arc-20160816; b=WwaqdD/1VuIVF02RCXR4Mv4aCMA6JnC7iC/n+IRlQrszlt/AbrFEQ026WjFXX042rG Ib+BUTny/TD0y+/TYEor4tcKfglqez2N944GjwGyxtfiZf+zXRBQ9/gYuOaTOaGUdsHc VfVh3XuA7oRQbaZGg6SrkBBZuRfzWzo+aR6AmiOvmtsYjuW1zw680QpL8AjP/hOSXteO pHDcdOc22289AKM6fSLJUe2n1rYTSHzg4oAr75qKx4XoUu3OjUSMAIEOPAuA5BJratz3 IlJ+J3yBjaWchg+bik44vquTKDTPJACNitOtzjtqW+gMCrMPCISgzyV2/nG8d03x9tBk Z+og== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-disposition:mime-version:list-unsubscribe:list-subscribe :list-id:precedence:message-id:subject:cc:to:from:date :dkim-signature; bh=33nfQ6E2IZzcs7Pxb0qMF/g8B61uICxiGHNN3IFmu48=; fh=CyjLWU9lN9EVz5MjLGz92OKwKHkIhvf/t4kvmyA+7eA=; b=Ecj4VxiWjT7oDDWnGcgtVbNmMtIx0UDD1T6A5WHAOSlCBzprZo+JxzJgxPyN5kNRCT DaWCjA/kKrDuJT1odesi5uZ+jaEig6O1KrI7Ms6RZusHhyxzmoL8L5HL025kD4UEd36g pSz72sg+BvGF3+tor0ekMgqeQAWV/Zyv7s4qmJDgprY0Fk3VIR1r/DC6SrKYtQNhntq/ 8CKGmmPdB31ectqXgdDzFu7nOMUJbA7Z1c2GD/kSEFJYv6BhWoFf/iQnZ6uFVHXvLFzM XcJZv8j0o81Lhs8Bv9TKZlx1DUFVJnCLFBguBVZZJm0AAQpRjSfDmOYsxwgL5nJvNpZM NBeA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=jEr8eJOH; arc=pass (i=1 spf=pass spfdomain=linaro.org dkim=pass dkdomain=linaro.org dmarc=pass fromdomain=linaro.org); spf=pass (google.com: domain of linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org X-Forwarded-Encrypted: i=2; AJvYcCW4FkwLp6rQ2yDqAPcvsGjvvG4zG2v0jzOY63gbfWgWYtl282fSdtDmecK7D89stm9rZ46tRAr1RAEls9LqNpl+gYy6wA== Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id c6-20020a056214224600b0068ef5373940si1150652qvc.335.2024.02.13.10.10.50 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Feb 2024 10:10:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=jEr8eJOH; arc=pass (i=1 spf=pass spfdomain=linaro.org dkim=pass dkdomain=linaro.org dmarc=pass fromdomain=linaro.org); spf=pass (google.com: domain of linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-64059-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 8293B1C236CA for <ouuuleilei@gmail.com>; Tue, 13 Feb 2024 18:10:21 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id A51E66086F; Tue, 13 Feb 2024 18:09:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="jEr8eJOH" Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8823605A3 for <linux-kernel@vger.kernel.org>; Tue, 13 Feb 2024 18:09:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.50 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707847789; cv=none; b=HINP4xWGvuKBRaCZUGTV2JlEHI7bqnxs1zkc8qDjyJsyCvKs3pQEUppDsHs3QTYkhbJoDKGPJFIy3eQz6hN9TvR6qhqrYQSl2ElLn0OKFJXasEouoHryRfnSCBuNXSg0xSW3F9MngDE3mhfxvzIV1GCd5BKazSczMFB9azUkCXE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707847789; c=relaxed/simple; bh=4ul5Tw3COSLN/gf7enKgp1nS0J788ARuuwxLOzxXNKg=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=D9BK+JrYCxzK/IcCuynWEYUMuKgzzTCYx7dOR7z/kIqmnr1lvfxw/buSTwXGIyfQxff+SPZkEeVkPzhPP2w7p8pqnceZ1ZjoRYpds7L64iXG2FGEIoKfAOPyuNSwz0COkwEKohhCM8kRq4MtcArVcPREU6quk5uww9G76H8zKtQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=jEr8eJOH; arc=none smtp.client-ip=209.85.208.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-558f523c072so7125172a12.2 for <linux-kernel@vger.kernel.org>; Tue, 13 Feb 2024 10:09:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1707847785; x=1708452585; darn=vger.kernel.org; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=33nfQ6E2IZzcs7Pxb0qMF/g8B61uICxiGHNN3IFmu48=; b=jEr8eJOHGPuv9ePsH2/x1K6QMulDrlHaVaqW+3OEwXzZncth6plYvGdNfQktzOiqgQ 5rdJGqQBl6WOuvnA130CWQAafxRvJdo4DrtZAcwe5o0gMHvcHqK/5YzvtEuCEXkT3f4W bUWQnAdVsZ0ew/psMP38XxJkyHSMrM5YZ1K0kjhPSkPGi4jxAfqfIBaFwMAGQf8UQCNz 585gcgJMByrkRh5NfIchzhle9vvwS7UNzchOsnznUALV/7D2YSHPvadLg4ApXoGC8LgE I4e/4i2DQbQ2AVvPjvP9wgwOhMnQET7/Nsl33EGZpIigD8Zw1vcKhD975S9UqhgqyJxe tRAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707847785; x=1708452585; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=33nfQ6E2IZzcs7Pxb0qMF/g8B61uICxiGHNN3IFmu48=; b=neJ+DH/95yr2sEQr0vQH03xed2thXA6NDkxxQby2KvZjGksDd00El0ymFzgsDdET3D IuZcMPpVn9Biw0BYJ931jMW3togrPaDQFZTEZYsx80RyLdh4VWZ5bFbhTNUJy1t4CgY6 vpgSiE4J55I4PsHFdXM+H8mTinBMenJGHPhG5y4rzTBMhDaNhqkpVEl7oYLYhQB0q1uJ FsOiCFdqaEefN/LaQUUorKIRT+pyt538teHaNZ2Fskr8Y1FWcfXuY2yufk0TTU6rBgGr 8d0EhlCe6W0l4ghUVmIahACa6ChuGseXyw39QFgYjP2fAK/GduFFSEs0Bcpj44GGtEGi 8+1g== X-Forwarded-Encrypted: i=1; AJvYcCU61/NQjG0hmIbP8iyGG91H2QXmdhKrRR+MykVsDE+NLOM8gNQ/MV3G+4wYRRr4NyWSOO2CmaqKg8cGdN6SDaFBYXEr4kf4+XU7Xjp/ X-Gm-Message-State: AOJu0Yxo6tXH/dU/3aVUvyxevBUNGcoGr7SwuLny+tXVDsbL0fVbRtbZ 0xI5zpriN0UXhHfsdh8dl1taTXmOV2gwkuPUt7LoPTWnpUY/iezOzkYefQSYFZ4= X-Received: by 2002:a17:906:f9c9:b0:a3c:f71e:215c with SMTP id lj9-20020a170906f9c900b00a3cf71e215cmr94770ejb.19.1707847785142; Tue, 13 Feb 2024 10:09:45 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCXtvq2ePchqRVLeWfWRgjTIk0XHuXOVDZfnCNszJq0nbh4i9MvodQpq3+8ABQI2MxEcHkYz5Scv6UcH/V3N+y/xplTvs75cSdcIXhTHTMTtw27UVIi0YBCIkkYkaWgKQ0GQCgemeQCjQr6wOD/pUUBCfyJTh45toXbSxkECFAdQoj/zayjtgIqsV+LiOukPpn43nbhAMz1p4mHYHrOyvcVydnqgRcM3NhvOpgxMvbvgxaK/ecksL/FqjmsT3v3EBsUd8jZDUcKZ5xd+PgL4Q7eJZcgFRfWJCd7I1AWjL3UQ2luNhrHp9QO5YoQhez+mhpPq94YvzCM30QWAnTzOsoioMInxZMNSMNxeKuHbzNkMgBR2pk7mjDJ9nTxHYlWlsE5b0ZX9Atdv/XGAZT/TOt6C1QYan6Mg/+1/bo8pgXhb+ZTcjVk6LfK3qQ== Received: from localhost ([102.222.70.76]) by smtp.gmail.com with ESMTPSA id gl15-20020a170906e0cf00b00a367bdce1fcsm1520638ejb.64.2024.02.13.10.09.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Feb 2024 10:09:44 -0800 (PST) Date: Tue, 13 Feb 2024 21:09:41 +0300 From: Dan Carpenter <dan.carpenter@linaro.org> To: Damian Muszynski <damian.muszynski@intel.com> Cc: Giovanni Cabiddu <giovanni.cabiddu@intel.com>, Herbert Xu <herbert@gondor.apana.org.au>, "David S. Miller" <davem@davemloft.net>, Lucas Segarra Fernandez <lucas.segarra.fernandez@intel.com>, Tero Kristo <tero.kristo@linux.intel.com>, Dan Carpenter <dan.carpenter@linaro.org>, Markas Rapoportas <markas.rapoportas@intel.com>, qat-linux@intel.com, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH] crypto: qat - uninitialized variable in adf_hb_error_inject_write() Message-ID: <193d36b0-961a-4b66-b945-37988f157ebe@moroto.mountain> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Mailer: git-send-email haha only kidding X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1790808267307578683 X-GMAIL-MSGID: 1790808267307578683 |
Series |
crypto: qat - uninitialized variable in adf_hb_error_inject_write()
|
|
Commit Message
Dan Carpenter
Feb. 13, 2024, 6:09 p.m. UTC
There are a few issues in this code. If *ppos is non-zero then the
first part of the buffer is not initialized. We never initialize the
last character of the buffer. The return is not checked so it's
possible that none of the buffer is initialized.
This is debugfs code which is root only and the impact of these bugs is
very small. However, it's still worth fixing. To fix this:
1) Check that *ppos is zero.
2) Use copy_from_user() instead of simple_write_to_buffer().
3) Explicitly add a NUL terminator.
Fixes: e2b67859ab6e ("crypto: qat - add heartbeat error simulator")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
.../crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
Comments
On Tue, Feb 13, 2024 at 09:09:41PM +0300, Dan Carpenter wrote: > There are a few issues in this code. If *ppos is non-zero then the > first part of the buffer is not initialized. We never initialize the > last character of the buffer. The return is not checked so it's > possible that none of the buffer is initialized. Thanks Dan. > > This is debugfs code which is root only and the impact of these bugs is > very small. However, it's still worth fixing. To fix this: > 1) Check that *ppos is zero. > 2) Use copy_from_user() instead of simple_write_to_buffer(). > 3) Explicitly add a NUL terminator. > > Fixes: e2b67859ab6e ("crypto: qat - add heartbeat error simulator") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > .../crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c b/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c > index 5cd6c2d6f90a..cccdff24b48d 100644 > --- a/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c > +++ b/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c > @@ -160,16 +160,17 @@ static ssize_t adf_hb_error_inject_write(struct file *file, > size_t count, loff_t *ppos) > { > struct adf_accel_dev *accel_dev = file->private_data; > - size_t written_chars; > char buf[3]; > int ret; > > /* last byte left as string termination */ > - if (count != 2) > + if (*ppos != 0 || count != 2) Is this alone not sufficient to fix the problem? Probably I'm missing something. The function just checks the first character in buf. Anyway, looks correct to me. Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> Regards,
On Fri, Feb 16, 2024 at 05:22:53PM +0000, Cabiddu, Giovanni wrote: > > --- a/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c > > +++ b/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c > > @@ -160,16 +160,17 @@ static ssize_t adf_hb_error_inject_write(struct file *file, > > size_t count, loff_t *ppos) > > { > > struct adf_accel_dev *accel_dev = file->private_data; > > - size_t written_chars; > > char buf[3]; > > int ret; > > > > /* last byte left as string termination */ > > - if (count != 2) > > + if (*ppos != 0 || count != 2) > Is this alone not sufficient to fix the problem? Probably I'm missing > something. > The function just checks the first character in buf. I mean, technically, yes. But leaving the last character uninitialized is ugly... Using simple_write_to_buffer() was inappropriate because it's not like this code supported partial writes. Better to just fix it all the way so no one copy and pastes it somewhere else. > > Anyway, looks correct to me. > Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> Thanks! regards, dan carpenter
diff --git a/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c b/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c index 5cd6c2d6f90a..cccdff24b48d 100644 --- a/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c +++ b/drivers/crypto/intel/qat/qat_common/adf_heartbeat_dbgfs.c @@ -160,16 +160,17 @@ static ssize_t adf_hb_error_inject_write(struct file *file, size_t count, loff_t *ppos) { struct adf_accel_dev *accel_dev = file->private_data; - size_t written_chars; char buf[3]; int ret; /* last byte left as string termination */ - if (count != 2) + if (*ppos != 0 || count != 2) return -EINVAL; - written_chars = simple_write_to_buffer(buf, sizeof(buf) - 1, - ppos, user_buf, count); + if (copy_from_user(buf, user_buf, count)) + return -EFAULT; + buf[count] = '\0'; + if (buf[0] != '1') return -EINVAL; @@ -183,7 +184,7 @@ static ssize_t adf_hb_error_inject_write(struct file *file, dev_info(&GET_DEV(accel_dev), "Heartbeat error injection enabled\n"); - return written_chars; + return count; } static const struct file_operations adf_hb_error_inject_fops = {