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;
Date:   Sat, 22 Jan 2022 11:46:15 -0800
From:   Colin Foster <colin.foster@in-advantage.com>
To:     Jesper Dangaard Brouer <jbrouer@redhat.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        brouer@redhat.com, LKML <linux-kernel@vger.kernel.org>,
        Network Development <netdev@vger.kernel.org>,
        Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>
Subject: Re: [net RFC v1 1/1] page_pool: fix NULL dereference crash
Message-ID: <20220122194615.GA914030@euler>
References: <20220122005644.802352-1-colin.foster@in-advantage.com>
 <20220122005644.802352-2-colin.foster@in-advantage.com>
 <CAADnVQK8xrQ92+=wm8AoDkC93SEKz3G=CoOnkPgvs=spJk5UZA@mail.gmail.com>
 <20220122024051.GA905413@euler>
 <5bd8f1bd-1a21-df1b-6d6f-9fe5657fdd7c@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5bd8f1bd-1a21-df1b-6d6f-9fe5657fdd7c@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?g37wnPWBzgnlt0gKipfxvkxOrng65aP28eRftOCIQhtItSSaE/hs483unb/A?=
 =?us-ascii?Q?Hk4Yf/KS99JYBhCrfz9bWM9L7AMxK9oKeTV8lvBEgGwA/ArEYp4wC6t1n/Bd?=
 =?us-ascii?Q?1pB0AewCMcShR7820L4DibTLyINabR1QKmoRkkO15/sgLkIfQwy2eM/RaoQh?=
 =?us-ascii?Q?UAOUqUPUM6/2ruUDTncRQ6USnwWi5EhD8E3PG38y2HOBbBYKgl4tDOJG7WbU?=
 =?us-ascii?Q?7eUuAY3ksmcbMG5Z/EYOxesLVhpaYfPORKtQ985Wzc1kHRioDgyg0l8mDLry?=
 =?us-ascii?Q?0EIUb7qdW8piVzFz4CgTsLI8rlPst9u4mfF5ImiS1F8CeHs5XE3UxlRvP9Dy?=
 =?us-ascii?Q?xf4gQIUNW5L8rgQU/szJmiBKar0VU2GmPDJtO6349T1zEu6K5/amSCdNfPXa?=
 =?us-ascii?Q?dd/+pQ4k99b/sA0CoTDuu4s8pbBxVjsGBqFgGWbivqoobe80Derb3f/IJMmN?=
 =?us-ascii?Q?mgRfpbCm/YV9Y2EYGdWqdFmHkw+Fotm/E91bCSkDiTPs7M9HHddVlySBLJQY?=
 =?us-ascii?Q?69X76b5NpzQpyKc+2g5fcHSBLE218qe+kbf2Oz7dN8GAHmMsl7klPcIRDO/9?=
 =?us-ascii?Q?nNE647DmeB0WDZQ6GNNyZAXurLvwOSwBslwpacq1klOMPm9dnkVmaBf/lbBt?=
 =?us-ascii?Q?qkDRO+UbqjEL6mb1bj5aAG0WWq5JT+9brR4rU7nMzKPbJ+6FIB2I1V/W4L6L?=
 =?us-ascii?Q?G+MmITqXbTVO6a+wWvWVdufxpAjUY5+/sfssps0MZ2V6RWw6tD//Jt1yGTuJ?=
 =?us-ascii?Q?Y/yRNtI/QS2dY7tBbQlLuKD99O73TJZman+9RJwNDtPrf7YwMln7o/JYhZ3r?=
 =?us-ascii?Q?NFHBHjrhEuqp7O2G+mMYZ+xgU9GLT2J3FQbAfcMrwaKzuGSsYGdtZgrLvVj+?=
 =?us-ascii?Q?bkRwmaveUE76yZwiKVvLDYKsp1PMtV6KVabl+l0tM1V3y+VSo5q4VHkaJJFB?=
 =?us-ascii?Q?MBJ4Ps1XKVON6uOOVgfmHiO74+62amoau88DkxwVJiP4V+kHpRWGNTRy4f+S?=
 =?us-ascii?Q?1HtswftGxdaGzf3MDlmxPgBTuPfEOEmcwwYjE7tXECkOGAGDDrMhsCNh0l4L?=
 =?us-ascii?Q?3LnXVdcT90M9zkYLEeFNYje1ELgEwquRqP4DlE0c+LkaEug/ZRadgcziX3qX?=
 =?us-ascii?Q?JMUnXzmSldjbu6m31rjMkW4w9QsQ7StR3tU33F9MfULY/pWJ9OJalXUMce7W?=
 =?us-ascii?Q?l7H2Ft20TPL7FxymnYsJlPY7F4urZTIcIlE5jSDJ0NVIx+mG5a6srjBAUToA?=
 =?us-ascii?Q?g1psbafvp+9KP2b1rYDsMJOJXButzjQRHsR15FspcdogAKENC8KNJ5wu8C1V?=
 =?us-ascii?Q?nWRpFTQErwvfs0GnLruAWqso8hkptHZuxI6fvweu/YnKk37bvSdPNu+UmB6k?=
 =?us-ascii?Q?k6BWu3ztDg8QxlK5mDU1KT4tLL3a0g9JqZJPwiu2g8bpCPZ/cDkDvjbKKqXZ?=
 =?us-ascii?Q?jwtAX0hCzwBS8/DbAZD4oa+f3QnbuNNkdD5FMjIhmQMZEtD/6NUNjExBWrfi?=
 =?us-ascii?Q?cxLBVvfSLrB8lronWAvWBYdMApsyPtX5F2aNSwoBoFUxVlkVgBKO6/L+Y/IV?=
 =?us-ascii?Q?qWQDOgoa6Auo+/eSY1xhf25WLyFaBayLzTEf4L2mlQJ1HdNLQh2+W1scdTfW?=
 =?us-ascii?Q?S6Bi3tpToBvp21nmfITdf2HDbGYaHuBP3tTifIdqmPXps62uge3a794NrRBu?=
 =?us-ascii?Q?ItzeDg=3D=3D?=
X-OriginatorOrg: in-advantage.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0dbb78b4-6c39-4bfe-0f04-08d9dddfde29
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2351.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jan 2022 19:46:22.8063
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 48e842ca-fbd8-4633-a79d-0c955a7d3aae
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ca2aztkyd+0WzWlb2TlfL4H7yKvLXj+GKnW74P+yMOw5zx/6jQTxxRtsP05qn+uCCf7nRoNA6SRSlpnP8y7Nx5xsENU2VzmhJBHdbZxH748=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB2045
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Jan 22, 2022 at 09:31:17AM +0100, Jesper Dangaard Brouer wrote:
> 
> 
> On 22/01/2022 03.40, Colin Foster wrote:
> > On Fri, Jan 21, 2022 at 05:13:28PM -0800, Alexei Starovoitov wrote:
> > > On Fri, Jan 21, 2022 at 4:57 PM Colin Foster
> > > <colin.foster@in-advantage.com> wrote:
> > > > 
> > > > Check for the existence of page pool params before dereferencing. This can
> > > > cause crashes in certain conditions.
> > > 
> > > In what conditions?
> > > Out of tree driver?
> > > 
> > > > Fixes: 35b2e549894b ("page_pool: Add callback to init pages when they are
> > > > allocated")
> > > > 
> > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > > > ---
> > > >   net/core/page_pool.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > > > index bd62c01a2ec3..641f849c95e7 100644
> > > > --- a/net/core/page_pool.c
> > > > +++ b/net/core/page_pool.c
> > > > @@ -213,7 +213,7 @@ static void page_pool_set_pp_info(struct page_pool *pool,
> > > >   {
> > > >          page->pp = pool;
> > > >          page->pp_magic |= PP_SIGNATURE;
> > > > -       if (pool->p.init_callback)
> > > > +       if (pool->p && pool->p.init_callback)
> > 
> > And my apologies - this should be if (pool... not if (pool->p. kernelbot
> > will be sure to tell me of this blunder soon
> 
> Can you confirm if your crash is fixed by this change?

Yes, this is confirmed. I'd obviously like to make a more comprehensive
commit message - my main question is "is this an issue for all DSA
configurations?" Seemingly that is the case, but like I said, I'm
unfamiliar with this code. I'll see if I can get a better understanding
before sending the real patch early next week.

> 
> 
> > > >                  pool->p.init_callback(page, pool->p.init_arg);
> > > >   }
>