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: <20230802192839.GB231007@hirez.programming.kicks-ass.net>
Date:   Wed, 2 Aug 2023 21:28:39 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
Cc:     "levymitchell0@...il.com" <levymitchell0@...il.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "mikelly@...rosoft.com" <mikelly@...rosoft.com>,
        Boqun Feng <boqun.feng@...il.com>
Subject: Re: [PATCH] hv_balloon: Update the balloon driver to use the SBRM API

On Wed, Aug 02, 2023 at 05:47:55PM +0000, Michael Kelley (LINUX) wrote:
> From: Mitchell Levy via B4 Relay <devnull+levymitchell0.gmail.com@...nel.org> Sent: Tuesday, July 25, 2023 5:24 PM
> >
> > This patch is intended as a proof-of-concept for the new SBRM
> > machinery[1]. For some brief background, the idea behind SBRM is using
> > the __cleanup__ attribute to automatically unlock locks (or otherwise
> > release resources) when they go out of scope, similar to C++ style RAII.
> > This promises some benefits such as making code simpler (particularly
> > where you have lots of goto fail; type constructs) as well as reducing
> > the surface area for certain kinds of bugs.
> > 
> > The changes in this patch should not result in any difference in how the
> > code actually runs (i.e., it's purely an exercise in this new syntax
> > sugar). In one instance SBRM was not appropriate, so I left that part
> > alone, but all other locking/unlocking is handled automatically in this
> > patch.
> > 
> > Link: https://lore.kernel.org/all/20230626125726.GU4253@hirez.programming.kicks-ass.net/ [1]
> 
> I haven't previously seen the "[1]" footnote-style identifier used with the
> Link: tag.  Usually the "[1]" goes at the beginning of the line with the
> additional information, but that conflicts with the Link: tag.  Maybe I'm
> wrong, but you might either omit the footnote-style identifier, or the Link:
> tag, instead of trying to use them together.
> 
> Separately, have you built a kernel for ARM64 with these changes in
> place?  The Hyper-V balloon driver is used on both x86 and ARM64
> architectures.  There's nothing obviously architecture specific here,
> but given that SBRM is new, it might be wise to verify that all is good
> when building and running on ARM64.

The only issue that has popped up so far is that __cleanup__ and
asm-goto don't interact nicely. GCC will silently mis-compile but clang
will issue a compile error/warning.

Specifically, GCC seems to have implemented asm-goto like the 'labels as
values' extention and loose the source context of the edge or something.
With result that the actual goto does not pass through the __cleanup__.

Other than that, it seems to work as expected across the platforms.

A brief look through the patch didn't show me anything odd, should be
ok. Although my primary purpose was to get rid of 'unlock' labels and
error handling, simple usage like this is perfectly fine too.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ