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;
Subject: Re: [PATCH] Using a pointer and kzalloc in place of a struct directly
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     andriin@fb.com, ast@kernel.org, bpf@vger.kernel.org,
        daniel@iogearbox.net, davem@davemloft.net, hawk@kernel.org,
        john.fastabend@gmail.com, kafai@fb.com, kpsingh@chromium.org,
        kuba@kernel.org, linux-kernel@vger.kernel.org
References: <000000000000c82fe505aef233c6@google.com>
 <20200912113804.6465-1-anant.thazhemadam@gmail.com>
 <20200912114706.GA171774@kroah.com>
From:   Anant Thazhemadam <anant.thazhemadam@gmail.com>
Message-ID: <09477eb1-bbeb-74e8-eba9-d72cce6104db@gmail.com>
Date:   Sat, 12 Sep 2020 17:43:38 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200912114706.GA171774@kroah.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 12/09/20 5:17 pm, Greg KH wrote:
> Note, your "To:" line seemed corrupted, and why not cc: the bpf mailing
> list as well?
Oh, I'm sorry about that. I pulled the emails of all the people to whom
this mail was sent off from the header in lkml mail, and just cc-ed
everyone.

> You leaked memory :(
>
> Did you test this patch?  Where do you free this memory, I don't see
> that happening anywhere in this patch, did I miss it?

Yes, I did test this patch, which didn't seem to trigger any issues.
It surprised me so much, that I ended up sending it in, to have
it checked out.

I wasn't sure where exactly the memory allocated here was
supposed to be freed (might be why the current implementation
isn't exactly using kzalloc). I forgot to mention it in the initial mail,
and I was hoping that someone would point me in the right direction
(if this approach was actually going to be considered, that is, which in
retrospect I now feel might not be the best thing)

> And odds are this change will slow things down, right?  Why make this
> change, what's wrong with the structure being on the stack?

For more clarity, I'm not exactly pushing for this patch to get accepted,
as much as I'm trying to understand what exactly is going on, and maybe
even understand syzbot's working a little better in the process.

At the time when I did send in this patch, the error seemed to be
present as far as syzbot was concerned. (I had sent in a test request not
too long before I sent this in, which returned a positive).
I just wanted to know, in the off-chance that the commit fix that was
pointed out wasn't merged in the tree yet when syzbot tested it, why
exactly would a patch like this lead to no issues getting triggered?
(I understand that if the fix was in the tree when syzbot ran the next test,
this patch immediately is rendered obsolete, ofcourse)

It felt somewhat a bit like an anomaly to me, and I figured it might be
worth investigating, is all; and I'd either infer something about syzbot,
or about whatever just happened there.

Now that I say it out loud, I realize it might sound a little silly, but
then again, I had tested the 'validity' of the bug, not too long before I
sent in the patch for syzbot to test too, and it seemed to be present when I did.

Thanks,
Anant