Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <6af0a6562b67a24e6233ed360189ba8071243035.camel@HansenPartnership.com>
Subject: Re: [PATCH] RDMA: usnic: Fix misuse of sysfs_emit_at
From:   James Bottomley <James.Bottomley@HansenPartnership.com>
To:     Joe Perches <joe@perches.com>,
        Christian Benvenuti <benve@cisco.com>,
        Nelson Escobar <neescoba@cisco.com>,
        Doug Ledford <dledford@redhat.com>,
        Jason Gunthorpe <jgg@ziepe.ca>
Cc:     linux-rdma@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        Greg KH <greg@kroah.com>
Date:   Fri, 15 Jan 2021 14:15:20 -0800
In-Reply-To: <f4ce30f297be4678634b5be4917401767ee6ebc5.camel@perches.com>
References: <f4ce30f297be4678634b5be4917401767ee6ebc5.camel@perches.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 2021-01-15 at 13:23 -0800, Joe Perches wrote:
> In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> _show
> uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to
> overwrite
> the last trailing space of potentially multiple entry output.
> 
> The length of the last sysfs_emit_at call is 1 and it should instead
> be
> ignored.  Do so.
> 
> Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> _show uses to sysfs_emit")
> 
> Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> index e59615a4c9d9..fc077855b46c 100644
> --- a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> +++ b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> @@ -231,7 +231,7 @@ static ssize_t summary_show(struct
> usnic_ib_qp_grp *qp_grp, char *buf)
>  		}
>  	}
>  
> -	len = sysfs_emit_at(buf, len, "\n");
> +	sysfs_emit_at(buf, len, "\n");	/* Overwrite the last
> trailing space */

len is the offset of where the next character gets written, isn't it?
so if you're overwriting the last character emitted into buf, shouldn't
the offset point at that character rather than one beyond it?  So

sysfs_emit_at(buf, len - 1, "\n");	/* Overwrite the last trailing
space */

?

James